Re: [RFA] Fix maint translate command

[Pedro Alves <pedro at codesourcery dot com>]

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