Patchwork [3/3] gdb/testsuite: Make use of exec_has_index_section function

login
register
mail settings
Submitter Andrew Burgess
Date Sept. 1, 2019, 4:08 p.m.
Message ID <8e6a86ffac28f50f57a826daf8cb4647e4de2ad3.1567353237.git.andrew.burgess@embecosm.com>
Download mbox | patch
Permalink /patch/34367/
State New
Headers show

Comments

Andrew Burgess - Sept. 1, 2019, 4:08 p.m.
Make use of exec_has_index_section library function rather than
manually checking in the 'maintenance info sections' output.  Should
make no difference to the test results, just makes the code easier to
read.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Use exec_has_index_section.
---
 gdb/testsuite/ChangeLog          |  4 ++++
 gdb/testsuite/gdb.base/maint.exp | 13 +------------
 2 files changed, 5 insertions(+), 12 deletions(-)
Tom de Vries - Sept. 2, 2019, 11:21 a.m.
On 01-09-19 18:08, Andrew Burgess wrote:
> Make use of exec_has_index_section library function rather than
> manually checking in the 'maintenance info sections' output.  Should
> make no difference to the test results, just makes the code easier to
> read.
> 

I agree that it's nicer to test this using a proc call.  I just wonder
whether we should move the removed code to the exec_has_index_section
proc to handle the case that there's no readelf. Then again, that might
be overkill, I'm not sure.

Thanks,
- Tom

> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/maint.exp: Use exec_has_index_section.
> ---
>  gdb/testsuite/ChangeLog          |  4 ++++
>  gdb/testsuite/gdb.base/maint.exp | 13 +------------
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> index a5d5dacaba9..15988c79386 100644
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -127,18 +127,7 @@ gdb_test_multiple "maint info sections" $test {
>  }
>  
>  # If we're using .gdb_index or .debug_names there will be no psymtabs.
> -set have_gdb_index 0
> -gdb_test_multiple "maint info sections .gdb_index .debug_names" "check for .gdb_index" {
> -    -re ": \\.gdb_index .*\r\n$gdb_prompt $" {
> -	set have_gdb_index 1
> -    }
> -    -re ": \\.debug_names .*\r\n$gdb_prompt $" {
> -	set have_gdb_index 1
> -    }
> -    -re ".*$gdb_prompt $" {
> -	;# Nothing to do, present to avoid a FAIL.
> -    }
> -}
> +set have_gdb_index [ exec_has_index_section ${binfile} ]
>  
>  # There also won't be any psymtabs if we read the index from the index cache.
>  # We can detect this by looking if the index-cache is enabled and if the number
>
Andrew Burgess - Sept. 2, 2019, 1:14 p.m.
* Tom de Vries <tdevries@suse.de> [2019-09-02 13:21:36 +0200]:

> On 01-09-19 18:08, Andrew Burgess wrote:
> > Make use of exec_has_index_section library function rather than
> > manually checking in the 'maintenance info sections' output.  Should
> > make no difference to the test results, just makes the code easier to
> > read.
> > 
> 
> I agree that it's nicer to test this using a proc call.  I just wonder
> whether we should move the removed code to the exec_has_index_section
> proc to handle the case that there's no readelf. Then again, that might
> be overkill, I'm not sure.

Possibly, but other functions in lib/gdb.exp don't provide non-readelf
alternatives.  Also, as readelf is in the same tree as GDB it feels
like it should be easy enough for any GDB developer to get access to
readelf.

I'm inclined to leave things as I have them, but if you (or anyone
else) feels strongly I'm happy to update the patch, just let me know.

Thanks,
Andrew

> 
> Thanks,
> - Tom
> 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/maint.exp: Use exec_has_index_section.
> > ---
> >  gdb/testsuite/ChangeLog          |  4 ++++
> >  gdb/testsuite/gdb.base/maint.exp | 13 +------------
> >  2 files changed, 5 insertions(+), 12 deletions(-)
> > 
> > diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
> > index a5d5dacaba9..15988c79386 100644
> > --- a/gdb/testsuite/gdb.base/maint.exp
> > +++ b/gdb/testsuite/gdb.base/maint.exp
> > @@ -127,18 +127,7 @@ gdb_test_multiple "maint info sections" $test {
> >  }
> >  
> >  # If we're using .gdb_index or .debug_names there will be no psymtabs.
> > -set have_gdb_index 0
> > -gdb_test_multiple "maint info sections .gdb_index .debug_names" "check for .gdb_index" {
> > -    -re ": \\.gdb_index .*\r\n$gdb_prompt $" {
> > -	set have_gdb_index 1
> > -    }
> > -    -re ": \\.debug_names .*\r\n$gdb_prompt $" {
> > -	set have_gdb_index 1
> > -    }
> > -    -re ".*$gdb_prompt $" {
> > -	;# Nothing to do, present to avoid a FAIL.
> > -    }
> > -}
> > +set have_gdb_index [ exec_has_index_section ${binfile} ]
> >  
> >  # There also won't be any psymtabs if we read the index from the index cache.
> >  # We can detect this by looking if the index-cache is enabled and if the number
> >
Tom de Vries - Sept. 2, 2019, 1:26 p.m.
On 02-09-19 15:14, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-09-02 13:21:36 +0200]:
> 
>> On 01-09-19 18:08, Andrew Burgess wrote:
>>> Make use of exec_has_index_section library function rather than
>>> manually checking in the 'maintenance info sections' output.  Should
>>> make no difference to the test results, just makes the code easier to
>>> read.
>>>
>>
>> I agree that it's nicer to test this using a proc call.  I just wonder
>> whether we should move the removed code to the exec_has_index_section
>> proc to handle the case that there's no readelf. Then again, that might
>> be overkill, I'm not sure.
> 
> Possibly, but other functions in lib/gdb.exp don't provide non-readelf
> alternatives.  Also, as readelf is in the same tree as GDB it feels
> like it should be easy enough for any GDB developer to get access to
> readelf.
> 
> I'm inclined to leave things as I have them, but if you (or anyone
> else) feels strongly I'm happy to update the patch, just let me know.

You've convinced me that it's overkill, thanks.
- Tom

Patch

diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index a5d5dacaba9..15988c79386 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -127,18 +127,7 @@  gdb_test_multiple "maint info sections" $test {
 }
 
 # If we're using .gdb_index or .debug_names there will be no psymtabs.
-set have_gdb_index 0
-gdb_test_multiple "maint info sections .gdb_index .debug_names" "check for .gdb_index" {
-    -re ": \\.gdb_index .*\r\n$gdb_prompt $" {
-	set have_gdb_index 1
-    }
-    -re ": \\.debug_names .*\r\n$gdb_prompt $" {
-	set have_gdb_index 1
-    }
-    -re ".*$gdb_prompt $" {
-	;# Nothing to do, present to avoid a FAIL.
-    }
-}
+set have_gdb_index [ exec_has_index_section ${binfile} ]
 
 # There also won't be any psymtabs if we read the index from the index cache.
 # We can detect this by looking if the index-cache is enabled and if the number