Re: [PATCH] Support bits/planes for ICON-resources proper

[Nicholas Clifton <nickc at redhat dot com>]

Hi Kai,

>   2014-10-27  Kai Tietz  <ktietz@redhat.com>
>
>       * resrc.c (define_cursor): Gain icon's planes/bits information
>       by the header-information.

A couple of minor points:

  * It would be nice if we could have a new test in the binutils 
testsuite that checks this patch.

  * I think that it would be cleaner if the value of "doff" was set 
only once, where the test is performed.  Ie:

> +      long doff = 8;
[...]
> +      /* Get the planes/bits by the bitmap header.
> +     See if we deal with a BITMAPCOREHEADER.  */
> +      if (get_long (e, real_filename) != 12)
> +    /* No, we deal with a BITMAPINFOHEADER.  */
> +    doff = 12;

  Could be:

         long doff;
	[...]
          /* See if we are dealing with a BITMAPCOREHEADER or a 
BITMAPINFOHEADER.  */
          doff = get_long (e, real_filename) == 12 ? 8 : 12;


Neither of these are sufficient to block the patch, so please consider 
it to be approved.  But if you have the time then the changes/additions 
would be nice.

Cheers
  Nick