Re: [patch] bfd - PE/COFF updates and clarifications

[Nick Clifton <nickc at redhat dot com>]

Hi Phil,

 I am very sorry that it has taken me so long to get around to 
reviewing this patch.

> > 2006-09-01  Phil Lello  <phil.lello@homecall.co.uk>
> >         * peXXigen.c: Updates for PE/COFF V8.0, and clarification
> >       (dir_names): Added CLR Runtime Header to dir_names[].
> >       (_bfd_XX_print_private_bfd_data_common): Added EFI_ROM and XBOX
> >       subsystem names
> >       (_bfd_XXi_swap_aouthdr_in, _bfd_XXi_swap_aouthdr_out)
> >       (pe_print_idata, pe_print_edata)
> >       (_bfd_XX_bfd_copy_private_bfd_data_common)
> >       (_bfd_XXi_final_link_postscript): Use #DEFINEs for index into
> >       DataDirectory

> > 2006-09-05  Phil Lello  <phil.lello@homecall.co.uk>
> >       * pe.h: Added defines for IMAGE_SUBSYSTEM_EFI_ROM and
> >       IMAGE_SUBSYSTEM_XBOX
> >       * internal.h: Added defines for PE directory entry types.
> >       NB: in internal.h because IMAGE_NUMBEROF_DIRECTORY_ENTRYIES is in
> >       pe.h

I have approved and applied your patch, except for one small part:

> > @@ -1214,7 +1214,8 @@ pe_print_idata (bfd * abfd, void * vfile
> >  	  ft_idx = first_thunk - adj;
> >  	  ft_allocated = 0; 
> >        
> > -	  if (first_thunk != hint_addr) 
> > +          // PCL HACK
> > +	  if (first_thunk != first_thunk/*first_thunk != hint_addr*/) 
> >  	    {
> >  	      /* Find the section which contains the first thunk.  */
> >  	      for (ft_section = abfd->sections;

It is not clear to me whether this change was intentional or just some 
debugging that was left in.  Either way it is unsuitable.  If you really 
do want to disable the test you should use "if (0)" and add a comment 
explaining why the test and the body of the if statement have been 
suppressed.

Cheers
  Nick