Re: [patch] readelf.c - dump .eh_frame sections

[Nick Clifton <nickc at redhat dot com>]

Hi DJ,

: 2000-11-27  DJ Delorie  <dj@redhat.com>
: 
: 	* readelf.c (usage): Add -wf (--debug-dump=frames) option.
: 	(parse_args): Support -wf option.
: 	(process_section_headers): Ditto.
: 	(debug_displays): Ditto.
: 	(display_debug_frames): New, dump *_frame sections.
: 	(frame_need_space): Support for above.
: 	(frame_display_row): Ditto.
: 	* binutils.texi: Document it.

Approved.

There are a few comments I would make though:

: +   /* DW_CFA_{undefined,same_value,offset,register} */

Two spaces at end of comment.

: +   unsigned int code_factor, data_factor;
: +   unsigned long pc_begin, pc_range;
: +   int cfa_reg, cfa_offset, ra;

Personally I prefer one field per line in a structure definition.  I
think that it makes the structure's contents easier to see.  But this
is just me.

: + static void
: + frame_display_row (Frame_Chunk *fc, int *need_col_headers, int *max_regs)
: + {

I believe that we are still trying to support compiling these tools
under K&R, so prototyped function declarations are out.

: + {
: +   int prev = fc->ncols;
: +   if (reg < fc->ncols)
: +     return;

I also like a blank line between variable declarations and the start
of a function's/block's code.  But again this is just a personal
preference.

: + 	      /* printf ("Invalid CIE pointer %08x in FDE at %08x\n", cie_id, saved_start); */

Why is this statement commented out ?  Also why a printf() rather than
a warn() ?

Cheers
	Nick