Re: patch: Updated windres tool Part 1 of 2

[Kai Tietz <Kai dot Tietz at onevision dot com>]

Hi Nick,

> Great - the patch is looking very good, but unfortunately there is still 

> one problem - I am getting some FAIL results in the windres part of the 
> binutils testsuite:
> 
>    FAIL: windres/capstyle (compare)
>    FAIL: windres/checkbox (compare)
>    FAIL: windres/combobox (compare)
>    FAIL: windres/deflang (compare)
>    FAIL: windres/dialog0 (compare)
>    FAIL: windres/dialog1 (compare)
>    FAIL: windres/dialogid (compare)
>    FAIL: windres/dialogsignature (compare)
>    FAIL: windres/edittext (compare)
>    FAIL: windres/escapex-2 (compare)
>    FAIL: windres/html (parse)
>    FAIL: windres/lang (compare)
>    FAIL: windres/listbox (compare)
>    FAIL: windres/messagetable (parse)
>    FAIL: windres/nocaption (compare)
>    FAIL: windres/printstyle (compare)
>    FAIL: windres/sublang (compare)
> 
> These are all happening because there are two zero-bytes missing from 
> the end of the objdump output in each of the tests.  eg from the 
> capstyle test:
> 
>    <  0070 4b000000 00000000                    K.......
>    >  0070 4b000000 0000                        K.....
>    FAIL: windres/capstyle (compare)

> Is the testsuite wrong or is the resource compiler not putting in enough 

> bytes ?

On one hand may not
        The last two zero bytes are reasoned by DWORD alignment and are 
not
        necessary for validity of the res file.
on the other hand
        Maybe we should align the last element to 4 bytes as it was done 
in
        the prior implementation.

I am really uncertain about this. The fix is quite simple by allocating 
the output buffer using a 4 bytes alignment in "resres.c".

> I also have one other comment, based on Dave Korn's observation:
> 
> >> I don't understand changes like this.  A bfd_vma represents a 
> >> memory address.  The base_style and default_style are IIUIC bitmasks
> >> of windows style flags.  It seems very wrong to use such an unrelated 

> >> type.
> 
> > I used the type bfd_vma for preventing people to think that 
rc_res_(XXX) 
> > structures are anyhow related to binary layout of these structures. 
> > Because this leads in fact to problems for different architectures 
with 
> > different base type sizes.
> 
> I understand that, but using bfd_vma is still very confusing.  It really 

> is meant to be a type for handling addresses.  Why not use bfd_size_type 

> instead ?  The underlying type will be the same as bfd_vma, but its name 

> is more open to non-address uses.
> 
> In fact even better would be to define your own type in terms of 
> bfd_size_type.  eg something like:
> 
>    /* Use bfd_size_type to ensure a sufficient number of bits.  */
>    typedef rc_bitfield_type bfd_size_type;
> 
> Then you can use rc_bitfield_type where you are currently using bfd_vma.

This sounds fine. I am agreeing to that. I will walk over the code for 
this. I would prefer as name something like rc_int_type. Is that ok ?

Cheers,
 i.A. Kai Tietz

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

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-StraÃe 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  KomplementÃrin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-StraÃe 9 â 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - GeschÃftsfÃhrer: 
Ulrike DÃhler, Manuela Kluger