[Ffmpeg-cvslog] r8608 - in trunk: Changelog doc/ffmpeg-doc.texi libavcodec/Makefile libavcodec/allcodecs.c libavcodec/avcodec.h libavcodec/tiff.c libavcodec/tiff.h libavcodec/tiffenc.c

[Michel Bardiaux]

Michel Bardiaux wrote:
> Michael Niedermayer wrote:
>> Hi
>>
>> On Wed, Apr 04, 2007 at 05:30:58PM +0200, Michel Bardiaux wrote:
>>> Uoti Urpala wrote:
>>>> On Wed, 2007-04-04 at 16:51 +0200, Michel Bardiaux wrote:
>>>>> Uoti Urpala wrote:
>>>>>> &s->width would be a pointer to int, which is wrong as a pointer to
>>>>>> uint32_t is needed.
>>>>> add_entry just needs void*.
>>>> No it needs a pointer to a 4 byte int. The parameter type of the
>>>> function is void *, but there are semantic requirements beyond that.
>>>> void * doesn't mean you could pass it any random pointer whatsoever and
>>>> expect it to do something sensible.
>>>>
>>> Sometimes its a 2-bytes unsigned. But you're right, &s->width would 
>>> not work if sizeof(int)!=4. I suppose the way to go is:
>>>
>>> uint32_t buf32;
>>> ...
>>> buf32 = s->width;
>>> add_entry(s, TIFF_WIDTH,TIFF_LONG,1,&buf32);
>>>
>>> Which is awkward, which explains why C9x introduced the construct.
>>
>> argh, i thought gcc 2.95 supports this, it does not :(
>> it only works if the array is dereferenced if its not (its address is 
>> used)
>> it fails
> 
> It seems to also work with constant initializers.
> 
>>
>> well how do we solve this
>> option A: make the entries in the struct match the correct uint* so we 
>> can
>> just take their address
>>
>> option B:
>> add_entry1(..., i){
>>     uint32_t dw=i;
>>     uint16_t w=i;
>>     add_entry(..., 1, type == TIFF_SHORT ? &w : &dw);
> 
> This is pseudocode, right?

I see now its not.

> 
>> }
>>
>> i think B is cleaner, feel free to fix it like that

I see you have fixed it yourself anyway.

> 
> As Uoti remarked, the requirements are semantic, this would be better 
> reflected using option A.
> 
> Greetings,


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/