Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]

On 9/24/21 2:21 PM, Pedro Alves wrote:
> On 2021-09-23 11:06 p.m., Tom de Vries via Gdb-patches wrote:
> 
>>  gdb/testsuite/gdb.testsuite/dump-system-info.exp | 48 ++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
>> new file mode 100644
>> index 00000000000..bf181469bd5
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
>> @@ -0,0 +1,48 @@
>> +# Copyright 2021 Free Software Foundation, Inc.
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# The purpose of this test-case is to dump /proc/cpuinfo and similar system
>> +# info into gdb.log.
>> +
>> +# Check if /proc/cpuinfo is available.
>> +set res [remote_exec target "test -r /proc/cpuinfo"]
>> +set status [lindex $res 0]
>> +set output [lindex $res 1]
> 

Hi,

thanks for the review.

> OOC, why "test -r" -> "cat" instead of "cat" straight away, which
> is basically what is done for the other dumps?
> 

The other cases are commands without file argument, this is a command
with file argument.  So, I was trying to not cause errors due to missing
file.

But you're right, it's not really necesssary.

> Consider factoring out a proc, like (untested, written in email):
> 

Copied from mail, and done ... which also fixes the typo you reported.

I'll commit this unless there are further comments.

Thanks,
- Tom



[-- Attachment #2: 0002-gdb-testsuite-Factor-out-dump_info-in-gdb.testsuite-dump-system-info.exp.patch --]
[-- Type: text/x-patch, Size: 2020 bytes --]

[gdb/testsuite] Factor out dump_info in gdb.testsuite/dump-system-info.exp

Factor out new proc dump_info, and in the process:
- fix a few typos
- remove unnecessary "test -r /proc/cpuinfo"

Tested on x86_64-linux.

Co-Authored-By: Pedro Alves <pedro@palves.net>

---
 gdb/testsuite/gdb.testsuite/dump-system-info.exp | 42 +++++++++---------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
index bf181469bd5..1831479265c 100644
--- a/gdb/testsuite/gdb.testsuite/dump-system-info.exp
+++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
@@ -15,34 +15,24 @@
 # The purpose of this test-case is to dump /proc/cpuinfo and similar system
 # info into gdb.log.
 
-# Check if /proc/cpuinfo is available.
-set res [remote_exec target "test -r /proc/cpuinfo"]
-set status [lindex $res 0]
-set output [lindex $res 1]
 
-if { $status == 0 && $output == "" } {
-    verbose -log "Cpuinfo available, dumping:"
-    remote_exec target "cat /proc/cpuinfo"
-} else {
-    verbose -log "Cpuinfo not available"
-}
-
-set res [remote_exec target "lsb_release -a"]
-set status [lindex $res 0]
-set output [lindex $res 1]
+proc dump_info {cmd {what ""}} {
 
-if { $status == 0 } {
-    verbose -log "lsb_release -a availabe, dumping:\n$output"
-} else {
-    verbose -log "lsb_release -a not available"
-}
+  if {$what == ""} {
+    set what $cmd
+  }
 
-set res [remote_exec target "uname -a"]
-set status [lindex $res 0]
-set output [lindex $res 1]
+  set res [remote_exec target $cmd]
+  set status [lindex $res 0]
+  set output [lindex $res 1]
 
-if { $status == 0 } {
-    verbose -log "uname -a availabe, dumping:\n$output"
-} else {
-    verbose -log "uname -a not available"
+  if { $status == 0 } {
+    verbose -log "$what available, dumping:\n$output"
+  } else {
+    verbose -log "$what not available"
+  }
 }
+
+dump_info "cat /proc/cpuinfo" "Cpuinfo"
+dump_info "uname -a"
+dump_info "lsb_release -a"