Re: [PATCH] Optimize reading ELF objects with many group sections

[Nick Clifton <nickc at redhat dot com>]

Hi Jens,

  A couple of comments on the patch:

  * It would be helpful if you could include a changelog 
    entry along with the patch itself.

  * Please can you tell us how you tested the patch, and if
    you noticed any regressions.  (I do not expect that there
    were any, but you never know).  In particular a patch like
    this that affects generic code needs to be tested with a 
    wide range of targets to make sure that it does not break
    anything.

  * Also...

> +++ b/bfd/elf-bfd.h
> @@ -1908,6 +1908,7 @@ struct elf_obj_tdata
>  
>    Elf_Internal_Shdr **group_sect_ptr;
>    int num_group;
> +  unsigned int group_search_offset;

  I think that it would be helpful to include a comment by 
  this field explaining what it is for.


> +	  /* Begin search from previous found group. */
> +	  unsigned i = j + search_offset;
> +
> +	  if (i >= num_group)
> +	    i -= num_group;

  I found this to be a little bit confusing when I was reading 
  the patch for the first time.  Maybe this would be clearer ?

          unsigned int i = (j + search_offset) % num_group;

  Of course since the intent is to optimise this loop for speed 
  you may object to the presence of the modulo operator, so I 
  leave the choice up to you.

Cheers
  Nick