[PING]RE: [RFA] Fix maint translate command

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

   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) \