This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: two patches for bugs in BFD/peXXigen.c


Thanks, two nits I see

2010/9/6 Alan Modra <amodra@gmail.com>:
> On Mon, Sep 06, 2010 at 10:55:08AM +0200, Kai Tietz wrote:
>> Would it be possible to get it without whitespace changes? 80% of it
>
> Index: bfd/peXXigen.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/peXXigen.c,v
> retrieving revision 1.64
> diff -u -p -w -r1.64 peXXigen.c
> @@ -1242,12 +1243,9 @@ pe_print_idata (bfd * abfd, void * vfile
> ? ? ? ? ? ? ?for (ft_section = abfd->sections;
> ? ? ? ? ? ? ? ? ? ft_section != NULL;
> ? ? ? ? ? ? ? ? ? ft_section = ft_section->next)
> - ? ? ? ? ? ? ? {
> - ? ? ? ? ? ? ? ? ft_datasize = ft_section->size;
> ? ? ? ? ? ? ? ? ?if (ft_addr >= ft_section->vma
> - ? ? ? ? ? ? ? ? ? ? && ft_addr < ft_section->vma + ft_datasize)
> + ? ? ? ? ? ? ? ? ? && ft_addr < ft_section->vma + ft_section->size)
> ? ? ? ? ? ? ? ? ? ?break;
> - ? ? ? ? ? ? ? }
I admit that this braces aren't really necessary, but it makes code
more readable to put this if statement into  {} block. The other
chance I see here to avoid this if completely would be to move the if
condition into the for's look condition block.

something like:

              for (ft_section = abfd->sections;
                   ft_section != NULL && ft_addr >= ft_section->vma
                   && ft_addr < ft_section->vma + ft_section->size;
                   ft_section = ft_section->next);

> @@ -1860,12 +1855,14 @@ _bfd_XX_print_ce_compressed_pdata (bfd *
> ? ? ? if (tsection && coff_section_data (abfd, tsection)
> ? ? ? ? ?&& pei_section_data (abfd, tsection))
> ? ? ? ?{
> - ? ? ? ? if (bfd_malloc_and_get_section (abfd, tsection, & tdata))
> - ? ? ? ? ? {
> ? ? ? ? ? ? ?int xx = (begin_addr - 8) - tsection->vma;
> + ? ? ? ? bfd_byte *tdata;
>
> ? ? ? ? ? ? ?tdata = (bfd_byte *) bfd_malloc (8);
> - ? ? ? ? ? ? if (bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8))
> + ? ? ? ? if (tdata)
> + ? ? ? ? ? {
> + ? ? ? ? ? ? if (bfd_get_section_contents (abfd, tsection,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? tdata, (bfd_vma) xx, 8))
> ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ?bfd_vma eh, eh_data;
>

Here I don't see the reason for using 'int' as type for xx. Why not
simply using here bfd_vma type and so no cast to this type is
necessary?

Kai

-- 
|? (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]