public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] Fix maint translate command
@ 2010-09-01  8:54 Pierre Muller
  2010-09-01 13:39 ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre Muller @ 2010-09-01  8:54 UTC (permalink / raw)
  To: gdb-patches

  'maint translate .text 0xXXXX'
failed saying that
 Unknown section .text.

  This problem is due to the introduction of
the macro ALL_OBJSECTIONS (objfile, sect)
in revision 1.67 of maint.c:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/maint.c.diff?cvsroot=src&r1=text&tr1=1.66&r2=text&tr2=1.67&f=u

  ALL_OBJSECTIONS is a double for loop,
thus the 'break' statement was only exiting the first loop,
but the second loop was still continuing, leading to 
a false error.

  Another option would be to simply revert that commit,
as the former code probably was working correctly
and is as nice as what I propose here.

  Checked on x86_64-unknown-linux-gnu (Compile Farm),
no regression found.

Pierre Muller
Pascal language support maintainer for GDB


projecttype:gdb
revision:HEAD
email:muller@ics.u-strasbg.fr
 
2010-09-01  Pierre Muller  <muller@ics.u-strasbg.fr>

	* maint.c (maintenance_translate_address): Fix search of named section.


Index: src/gdb/maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.79
diff -u -p -r1.79 maint.c
--- src/gdb/maint.c     1 Jul 2010 15:36:16 -0000       1.79
+++ src/gdb/maint.c     1 Sep 2010 07:43:56 -0000
@@ -452,7 +452,7 @@ static void
 maintenance_translate_address (char *arg, int from_tty)
 {
   CORE_ADDR address;
-  struct obj_section *sect;
+  struct obj_section *sect, *fsect;
   char *p;
   struct minimal_symbol *sym;
   struct objfile *objfile;
@@ -473,14 +473,26 @@ maintenance_translate_address (char *arg
       while (isspace (*p))
 	p++;			/* Skip whitespace */
 
-      ALL_OBJSECTIONS (objfile, sect)
-      {
-	if (strcmp (sect->the_bfd_section->name, arg) == 0)
-	  break;
-      }
+	fsect = NULL;
 
-      if (!objfile)
-	error (_("Unknown section %s."), arg);
+	ALL_OBJFILES (objfile)
+	  {
+	    ALL_OBJFILE_OSECTIONS (objfile, sect)
+	      {
+		if (strcmp (sect->the_bfd_section->name, arg) == 0)
+		  {
+		    fsect = sect;
+		    break;
+		  }
+	      }
+	    if (fsect)
+	      break;
+	  }
+  
+	if (!fsect)
+	  error (_("Unknown section %s."), arg);
+	else
+	  sect = fsect;
     }
 
   address = parse_and_eval_address (p);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Fix maint translate command
  2010-09-01  8:54 [RFA] Fix maint translate command Pierre Muller
@ 2010-09-01 13:39 ` Pedro Alves
  2010-09-01 13:49   ` Pierre Muller
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-09-01 13:39 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pierre Muller

On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote:
>   ALL_OBJSECTIONS is a double for loop,
> thus the 'break' statement was only exiting the first loop,
> but the second loop was still continuing, leading to 
> a false error.
> 

It all sounds like we should fix ALL_OBJSECTIONS then.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFA] Fix maint translate command
  2010-09-01 13:39 ` Pedro Alves
@ 2010-09-01 13:49   ` Pierre Muller
  2010-09-01 15:25     ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre Muller @ 2010-09-01 13:49 UTC (permalink / raw)
  To: 'Pedro Alves', gdb-patches

Honestly, this might be a good idea,
but I have no idea how to do this :(

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Wednesday, September 01, 2010 3:40 PM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Fix maint translate command
> 
> On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote:
> >   ALL_OBJSECTIONS is a double for loop,
> > thus the 'break' statement was only exiting the first loop,
> > but the second loop was still continuing, leading to
> > a false error.
> >
> 
> It all sounds like we should fix ALL_OBJSECTIONS then.
> 
> --
> Pedro Alves

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Fix maint translate command
  2010-09-01 13:49   ` Pierre Muller
@ 2010-09-01 15:25     ` Pedro Alves
  2010-09-01 16:07       ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-09-01 15:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pierre Muller

[please don't top-post]

On Wednesday 01 September 2010 14:49:30, Pierre Muller wrote:
> > On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote:
> > >   ALL_OBJSECTIONS is a double for loop,
> > > thus the 'break' statement was only exiting the first loop,
> > > but the second loop was still continuing, leading to
> > > a false error.
> > >
> > 
> > It all sounds like we should fix ALL_OBJSECTIONS then.

> Honestly, this might be a good idea,
> but I have no idea how to do this :(

With Magic.  It would be real simple if the inner loop's end condition
was a check againt NULL, rather than "(osect) < (objfile)->sections_end"
--- you'd just add an osect == NULL check to the outer loop's conditional.

Something like this is still simple, but clears "objfile" when you break
out of the inner loop:

#define ALL_OBJSECTIONS(objfile, osect)					\
  for ((objfile) = current_program_space->objfiles;			\
       (objfile) != NULL;						\
       (objfile) = (osect) < (objfile)->sections_end ? NULL : (objfile)->next) \
    for ((osect) = (objfile)->sections; (osect) < (objfile)->sections_end; osect++)


It happens that "maintenance translate ..." expects that OBJFILE is preserved
when you "break" within ALL_OBJSECTIONS, which, is probably what most would
expect if they didn't know the innards to the macro, so, we go a step further
to preserve that OBJFILE pointer:

#define ALL_OBJSECTIONS(objfile, osect)					\
  for ((objfile) = current_program_space->objfiles,			\
	 (osect) = (objfile) != NULL ? (objfile)->sections_end : NULL ; \
       (objfile) != NULL						\
	 && (osect) == (objfile)->sections_end;				\
       (objfile) = (osect) < (objfile)->sections_end			\
	 ? (objfile)							\
	 : (objfile)->next)						\
    for ((osect) = (objfile)->sections;					\
	 (osect) < (objfile)->sections_end;				\
	 osect++)

I admit it loop ugly, and that may look complicated, but it isn't
(complicated).  The outer loop learns about the inner loop's end
condition, and stops iterating if it detects the inner loop didn't
reach it's end.  The trick to not clearing "objfile" is to only
advance it in the outer loop, if the inner loop reached it's end.

It fixes "maint translate .text 0xXXX" for me and has no regressions on 
x86_64-unknown-linux-gnu.

There's a:

  goto keep;	/* break out of two nested for loops */

in gcore.c that could then be replaced with a "break" ...

-- 
Pedro Alves

2010-09-01  Pedro Alves  <pedro@codesourcery.com>

	* objfiles.h (ALL_OBJSECTIONS): Handle breaks in the inner loop.

---
 gdb/objfiles.h |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Index: src/gdb/objfiles.h
===================================================================
--- src.orig/gdb/objfiles.h	2010-09-01 14:35:08.000000000 +0100
+++ src/gdb/objfiles.h	2010-09-01 16:10:53.000000000 +0100
@@ -611,9 +611,24 @@ extern int gdb_bfd_close_or_warn (struct
 #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
   for (osect = objfile->sections; osect < objfile->sections_end; osect++)
 
-#define ALL_OBJSECTIONS(objfile, osect)		\
-  ALL_OBJFILES (objfile)			\
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
+/* Traverse all obj_sections in all objfiles in the current program
+   space.  Note that this detects a "break" in the inner loop, and
+   exits immediately from the outer loop as well, thus, client code
+   doesn't need to know that this is implemented with a double for.
+   The extra hair is to make sure that OBJFILE isn't cleared on a
+   "break".  */
+
+#define ALL_OBJSECTIONS(objfile, osect)					\
+  for ((objfile) = current_program_space->objfiles,			\
+	 (osect) = (objfile) != NULL ? (objfile)->sections_end : NULL ; \
+       (objfile) != NULL						\
+	 && (osect) == (objfile)->sections_end;				\
+       (objfile) = (osect) < (objfile)->sections_end			\
+	 ? (objfile)							\
+	 : (objfile)->next)						\
+    for ((osect) = (objfile)->sections;					\
+	 (osect) < (objfile)->sections_end;				\
+	 osect++)
 
 #define SECT_OFF_DATA(objfile) \
      ((objfile->sect_index_data == -1) \

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFA] Fix maint translate command
  2010-09-01 15:25     ` Pedro Alves
@ 2010-09-01 16:07       ` Pedro Alves
  2010-09-14 15:14         ` [PING]RE: " Pierre Muller
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-09-01 16:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pierre Muller

On Wednesday 01 September 2010 16:25:27, Pedro Alves wrote:
> [please don't top-post]
> 
> On Wednesday 01 September 2010 14:49:30, Pierre Muller wrote:
> > > On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote:
> > > >   ALL_OBJSECTIONS is a double for loop,
> > > > thus the 'break' statement was only exiting the first loop,
> > > > but the second loop was still continuing, leading to
> > > > a false error.
> > > >
> > > 
> > > It all sounds like we should fix ALL_OBJSECTIONS then.
> 
> > Honestly, this might be a good idea,
> > but I have no idea how to do this :(
> 
> With Magic.  It would be real simple if the inner loop's end condition
> was a check againt NULL, rather than "(osect) < (objfile)->sections_end"
> --- you'd just add an osect == NULL check to the outer loop's conditional.
> 
> Something like this is still simple, but clears "objfile" when you break
> out of the inner loop:
> 
> #define ALL_OBJSECTIONS(objfile, osect)					\
>   for ((objfile) = current_program_space->objfiles;			\
>        (objfile) != NULL;						\
>        (objfile) = (osect) < (objfile)->sections_end ? NULL : (objfile)->next) \
>     for ((osect) = (objfile)->sections; (osect) < (objfile)->sections_end; osect++)
> 
> 
> It happens that "maintenance translate ..." expects that OBJFILE is preserved
> when you "break" within ALL_OBJSECTIONS, which, is probably what most would
> expect if they didn't know the innards to the macro, so, we go a step further
> to preserve that OBJFILE pointer:
> 

Sorry, I left it half baked.  Here's the correct version (I forgot to
update "osect" when advancing the objfile in the outer loop).  I think
it's correct now:

#define ALL_OBJSECTIONS(objfile, osect)					\
  for ((objfile) = current_program_space->objfiles,			\
	 (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;	\
       (objfile) != NULL						\
	 && (osect) == (objfile)->sections_end;				\
       ((osect) == (objfile)->sections_end				\
	? ((objfile) = (objfile)->next,					\
	   (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)	\
	: 0))								\
    for ((osect) = (objfile)->sections;					\
	 (osect) < (objfile)->sections_end;				\
	 (osect)++)

> I admit it loop ugly, and that may look complicated, but it isn't
> (complicated).  The outer loop learns about the inner loop's end
> condition, and stops iterating if it detects the inner loop didn't
> reach it's end.  The trick to not clearing "objfile" is to only
> advance it in the outer loop, if the inner loop reached it's end.
> 
> It fixes "maint translate .text 0xXXX" for me and has no regressions on 
> x86_64-unknown-linux-gnu.
> 
> There's a:
> 
>   goto keep;	/* break out of two nested for loops */
> 
> in gcore.c that could then be replaced with a "break" ...

-- 
Pedro Alves

2010-09-01  Pedro Alves  <pedro@codesourcery.com>

	* objfiles.h (ALL_OBJSECTIONS): Handle breaks in the inner loop.

---
 gdb/objfiles.h |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Index: src/gdb/objfiles.h
===================================================================
--- src.orig/gdb/objfiles.h	2010-09-01 14:35:08.000000000 +0100
+++ src/gdb/objfiles.h	2010-09-01 16:58:36.000000000 +0100
@@ -611,9 +611,25 @@ extern int gdb_bfd_close_or_warn (struct
 #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
   for (osect = objfile->sections; osect < objfile->sections_end; osect++)
 
-#define ALL_OBJSECTIONS(objfile, osect)		\
-  ALL_OBJFILES (objfile)			\
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
+/* Traverse all obj_sections in all objfiles in the current program
+   space.  Note that this detects a "break" in the inner loop, and
+   exits immediately from the outer loop as well, thus, client code
+   doesn't need to know that this is implemented with a double for.
+   The extra hair is to make sure that OBJFILE isn't cleared on a
+   "break".  */
+
+#define ALL_OBJSECTIONS(objfile, osect)					\
+  for ((objfile) = current_program_space->objfiles,			\
+	 (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;	\
+       (objfile) != NULL						\
+	 && (osect) == (objfile)->sections_end;				\
+       ((osect) == (objfile)->sections_end				\
+	? ((objfile) = (objfile)->next,					\
+	   (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)	\
+	: 0))								\
+    for ((osect) = (objfile)->sections;					\
+	 (osect) < (objfile)->sections_end;				\
+	 (osect)++)
 
 #define SECT_OFF_DATA(objfile) \
      ((objfile->sect_index_data == -1) \

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PING]RE: [RFA] Fix maint translate command
  2010-09-01 16:07       ` Pedro Alves
@ 2010-09-14 15:14         ` Pierre Muller
  2010-09-22 18:59           ` Joel Brobecker
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre Muller @ 2010-09-14 15:14 UTC (permalink / raw)
  To: 'Pedro Alves', gdb-patches

   The problem of break inside ALL_OBJSECTIONS
is still not fixed on CVS despite a fix proposed by Pedro?

  I must confess that I am unable to fully understand such complicated
macros, and I leave to others the responsibility to
approve such kind of patches (I don't have any rights on
objfiles sources anyhow ...)

  Would it be possible to check Pedro's patch in?



Pierre Muller
Pascal language support maintainer for GDB




> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Wednesday, September 01, 2010 6:07 PM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Fix maint translate command
> 
> On Wednesday 01 September 2010 16:25:27, Pedro Alves wrote:
> > [please don't top-post]
> >
> > On Wednesday 01 September 2010 14:49:30, Pierre Muller wrote:
> > > > On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote:
> > > > >   ALL_OBJSECTIONS is a double for loop,
> > > > > thus the 'break' statement was only exiting the first loop,
> > > > > but the second loop was still continuing, leading to
> > > > > a false error.
> > > > >
> > > >
> > > > It all sounds like we should fix ALL_OBJSECTIONS then.
> >
> > > Honestly, this might be a good idea,
> > > but I have no idea how to do this :(
> >
> > With Magic.  It would be real simple if the inner loop's end
> condition
> > was a check againt NULL, rather than "(osect) < (objfile)-
> >sections_end"
> > --- you'd just add an osect == NULL check to the outer loop's
> conditional.
> >
> > Something like this is still simple, but clears "objfile" when you
> break
> > out of the inner loop:
> >
> > #define ALL_OBJSECTIONS(objfile, osect)					\
> >   for ((objfile) = current_program_space->objfiles;			\
> >        (objfile) != NULL;						\
> >        (objfile) = (osect) < (objfile)->sections_end ? NULL :
> (objfile)->next) \
> >     for ((osect) = (objfile)->sections; (osect) < (objfile)-
> >sections_end; osect++)
> >
> >
> > It happens that "maintenance translate ..." expects that OBJFILE is
> preserved
> > when you "break" within ALL_OBJSECTIONS, which, is probably what most
> would
> > expect if they didn't know the innards to the macro, so, we go a step
> further
> > to preserve that OBJFILE pointer:
> >
> 
> Sorry, I left it half baked.  Here's the correct version (I forgot to
> update "osect" when advancing the objfile in the outer loop).  I think
> it's correct now:
> 
> #define ALL_OBJSECTIONS(objfile, osect)					\
>   for ((objfile) = current_program_space->objfiles,			\
> 	 (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;
> 	\
>        (objfile) != NULL						\
> 	 && (osect) == (objfile)->sections_end;				\
>        ((osect) == (objfile)->sections_end				\
> 	? ((objfile) = (objfile)->next,					\
> 	   (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)
> 	\
> 	: 0))								\
>     for ((osect) = (objfile)->sections;					\
> 	 (osect) < (objfile)->sections_end;				\
> 	 (osect)++)
> 
> > I admit it loop ugly, and that may look complicated, but it isn't
> > (complicated).  The outer loop learns about the inner loop's end
> > condition, and stops iterating if it detects the inner loop didn't
> > reach it's end.  The trick to not clearing "objfile" is to only
> > advance it in the outer loop, if the inner loop reached it's end.
> >
> > It fixes "maint translate .text 0xXXX" for me and has no regressions
> on
> > x86_64-unknown-linux-gnu.
> >
> > There's a:
> >
> >   goto keep;	/* break out of two nested for loops */
> >
> > in gcore.c that could then be replaced with a "break" ...
> 
> --
> Pedro Alves
> 
> 2010-09-01  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* objfiles.h (ALL_OBJSECTIONS): Handle breaks in the inner loop.
> 
> ---
>  gdb/objfiles.h |   22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> Index: src/gdb/objfiles.h
> ===================================================================
> --- src.orig/gdb/objfiles.h	2010-09-01 14:35:08.000000000 +0100
> +++ src/gdb/objfiles.h	2010-09-01 16:58:36.000000000 +0100
> @@ -611,9 +611,25 @@ extern int gdb_bfd_close_or_warn (struct
>  #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
>    for (osect = objfile->sections; osect < objfile->sections_end;
> osect++)
> 
> -#define ALL_OBJSECTIONS(objfile, osect)		\
> -  ALL_OBJFILES (objfile)			\
> -    ALL_OBJFILE_OSECTIONS (objfile, osect)
> +/* Traverse all obj_sections in all objfiles in the current program
> +   space.  Note that this detects a "break" in the inner loop, and
> +   exits immediately from the outer loop as well, thus, client code
> +   doesn't need to know that this is implemented with a double for.
> +   The extra hair is to make sure that OBJFILE isn't cleared on a
> +   "break".  */
> +
> +#define ALL_OBJSECTIONS(objfile, osect)					\
> +  for ((objfile) = current_program_space->objfiles,			\
> +	 (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;
> 	\
> +       (objfile) != NULL						\
> +	 && (osect) == (objfile)->sections_end;				\
> +       ((osect) == (objfile)->sections_end				\
> +	? ((objfile) = (objfile)->next,					\
> +	   (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)
> 	\
> +	: 0))								\
> +    for ((osect) = (objfile)->sections;					\
> +	 (osect) < (objfile)->sections_end;				\
> +	 (osect)++)
> 
>  #define SECT_OFF_DATA(objfile) \
>       ((objfile->sect_index_data == -1) \

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PING]RE: [RFA] Fix maint translate command
  2010-09-14 15:14         ` [PING]RE: " Pierre Muller
@ 2010-09-22 18:59           ` Joel Brobecker
  2010-09-22 19:23             ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2010-09-22 18:59 UTC (permalink / raw)
  To: Pierre Muller; +Cc: 'Pedro Alves', gdb-patches

On Tue, Sep 14, 2010 at 10:10:02AM +0200, Pierre Muller wrote:
>    The problem of break inside ALL_OBJSECTIONS
> is still not fixed on CVS despite a fix proposed by Pedro?
> 
>   I must confess that I am unable to fully understand such complicated
> macros, and I leave to others the responsibility to
> approve such kind of patches (I don't have any rights on
> objfiles sources anyhow ...)
> 
>   Would it be possible to check Pedro's patch in?

Seems like everyone is really really busy these days :-(.
This is coming from Pedro, and I didn't see anything wrong with it.
Since we haven't received any objection, I think it is fine to commit
after running it against the testsuite.

> > > I admit it loop ugly, and that may look complicated, but it isn't
> > > (complicated).  The outer loop learns about the inner loop's end
> > > condition, and stops iterating if it detects the inner loop didn't
> > > reach it's end.  The trick to not clearing "objfile" is to only
> > > advance it in the outer loop, if the inner loop reached it's end.

I think that this is a useful comment to add to the macro.  And perhaps
explain that the purpose is to allow `break' to exit the ALL_OBJSECTIONS
(double-) loop.

-- 
Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PING]RE: [RFA] Fix maint translate command
  2010-09-22 18:59           ` Joel Brobecker
@ 2010-09-22 19:23             ` Pedro Alves
  2010-09-24 15:55               ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-09-22 19:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker, Pierre Muller

On Wednesday 22 September 2010 18:01:11, Joel Brobecker wrote:
> On Tue, Sep 14, 2010 at 10:10:02AM +0200, Pierre Muller wrote:
> >    The problem of break inside ALL_OBJSECTIONS
> > is still not fixed on CVS despite a fix proposed by Pedro?
> > 
> >   I must confess that I am unable to fully understand such complicated
> > macros, and I leave to others the responsibility to
> > approve such kind of patches (I don't have any rights on
> > objfiles sources anyhow ...)
> > 
> >   Would it be possible to check Pedro's patch in?
> 
> Seems like everyone is really really busy these days :-(.

Indeed.  :-(

> This is coming from Pedro, and I didn't see anything wrong with it.
> Since we haven't received any objection, I think it is fine to commit
> after running it against the testsuite.

I'll test&apply this tomorrow.  Sorry for the delay.

> > > > I admit it loop ugly, and that may look complicated, but it isn't
> > > > (complicated).  The outer loop learns about the inner loop's end
> > > > condition, and stops iterating if it detects the inner loop didn't
> > > > reach it's end.  The trick to not clearing "objfile" is to only
> > > > advance it in the outer loop, if the inner loop reached it's end.
> 
> I think that this is a useful comment to add to the macro.  And perhaps
> explain that the purpose is to allow `break' to exit the ALL_OBJSECTIONS
> (double-) loop.

I'll extend the current comment in that direction.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PING]RE: [RFA] Fix maint translate command
  2010-09-22 19:23             ` Pedro Alves
@ 2010-09-24 15:55               ` Pedro Alves
  0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2010-09-24 15:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker, Pierre Muller

On Wednesday 22 September 2010 19:59:03, Pedro Alves wrote:
> On Wednesday 22 September 2010 18:01:11, Joel Brobecker wrote:

> > I think that this is a useful comment to add to the macro.  And perhaps
> > explain that the purpose is to allow `break' to exit the ALL_OBJSECTIONS
> > (double-) loop.
> 
> I'll extend the current comment in that direction.

Done now.  Tested on x86_64-unknown-linux-gnu and applied.

-- 
Pedro Alves

2010-09-24  Pedro Alves  <pedro@codesourcery.com>

	* objfiles.h (ALL_OBJSECTIONS): Handle breaks in the inner loop.

---
 gdb/objfiles.h |   41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

Index: src/gdb/objfiles.h
===================================================================
--- src.orig/gdb/objfiles.h	2010-09-24 10:22:44.000000000 +0100
+++ src/gdb/objfiles.h	2010-09-24 11:32:44.000000000 +0100
@@ -611,9 +611,44 @@ extern int gdb_bfd_close_or_warn (struct
 #define ALL_OBJFILE_OSECTIONS(objfile, osect)	\
   for (osect = objfile->sections; osect < objfile->sections_end; osect++)
 
-#define ALL_OBJSECTIONS(objfile, osect)		\
-  ALL_OBJFILES (objfile)			\
-    ALL_OBJFILE_OSECTIONS (objfile, osect)
+/* Traverse all obj_sections in all objfiles in the current program
+   space.
+
+   Note that this detects a "break" in the inner loop, and exits
+   immediately from the outer loop as well, thus, client code doesn't
+   need to know that this is implemented with a double for.  The extra
+   hair is to make sure that a "break;" stops the outer loop iterating
+   as well, and both OBJFILE and OSECT are left unmodified:
+
+    - The outer loop learns about the inner loop's end condition, and
+      stops iterating if it detects the inner loop didn't reach its
+      end.  In other words, the outer loop keeps going only if the
+      inner loop reached its end cleanly [(osect) ==
+      (objfile)->sections_end].
+
+    - OSECT is initialized in the outer loop initialization
+      expressions, such as if the inner loop has reached its end, so
+      the check mentioned above succeeds the first time.
+
+    - The trick to not clearing OBJFILE on a "break;" is, in the outer
+      loop's loop expression, advance OBJFILE, but iff the inner loop
+      reached its end.  If not, there was a "break;", so leave OBJFILE
+      as is; the outer loop's conditional will break immediately as
+      well (as OSECT will be different from OBJFILE->sections_end).
+*/
+
+#define ALL_OBJSECTIONS(objfile, osect)					\
+  for ((objfile) = current_program_space->objfiles,			\
+	 (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0;	\
+       (objfile) != NULL						\
+	 && (osect) == (objfile)->sections_end;				\
+       ((osect) == (objfile)->sections_end				\
+	? ((objfile) = (objfile)->next,					\
+	   (objfile) != NULL ? (osect) = (objfile)->sections_end : 0)	\
+	: 0))								\
+    for ((osect) = (objfile)->sections;					\
+	 (osect) < (objfile)->sections_end;				\
+	 (osect)++)
 
 #define SECT_OFF_DATA(objfile) \
      ((objfile->sect_index_data == -1) \

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-09-24 11:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-01  8:54 [RFA] Fix maint translate command Pierre Muller
2010-09-01 13:39 ` Pedro Alves
2010-09-01 13:49   ` Pierre Muller
2010-09-01 15:25     ` Pedro Alves
2010-09-01 16:07       ` Pedro Alves
2010-09-14 15:14         ` [PING]RE: " Pierre Muller
2010-09-22 18:59           ` Joel Brobecker
2010-09-22 19:23             ` Pedro Alves
2010-09-24 15:55               ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).