2013/4/23 Rico Schüller <kgbric...@web.de>
> On 22.04.2013 23:08, Christian Costa wrote:
>
>> +typedef struct {
>> + DWORD bc2Size;
>> + DWORD bc2Width;
>> + DWORD bc2Height;
>> + WORD bc2Planes;
>> + WORD bc2BitCount;
>> + DWORD bc2Compression;
>> + DWORD bc2SizeImage;
>> + DWORD bc2XRes;
>> + DWORD bc2YRes;
>> + DWORD bc2ClrUsed;
>> + DWORD bc2ClrImportant;
>> + /* same as BITMAPINFOHEADER until this point */
>> + WORD bc2ResUnit;
>> + WORD bc2Reserved;
>> + WORD bc2Orientation;
>> + WORD bc2Halftoning;
>> + DWORD bc2HalftoneSize1;
>> + DWORD bc2HalftoneSize2;
>> + DWORD bc2ColorSpace;
>> + DWORD bc2AppData;
>> +} BITMAPCOREHEADER2;
>>
>
> Do we really need a typedef here? For only locally used structs, I think
> we shouldn't use a typedef. Also the struct is only used to determine the
> size and for nothing else? There may be a better way to do that... Maybe
> something like in dlls/windowscodecs/icoformat.c ?
>
> I think you got inspired by dlls/windowscodecs/bmpdecode.**c, there is
> the same issue. Why do we need a typedef, if we only use it in one file to
> calculate the size?
>
This is correct. Indeed using header_size == 64 /*
sizeof(BITMAPCOREHEADER2) */ would be better.
>
>
>> + BITMAPFILEHEADER *header;
>>
>>
>> + if ((header_size == sizeof(BITMAPINFOHEADER)) ||
>> + (header_size == sizeof(BITMAPV4HEADER)) ||
>> + (header_size == sizeof(BITMAPV5HEADER)) ||
>> + (header_size == sizeof(BITMAPCOREHEADER2)))
>> + {
>> + /* All structures have the same memory layout as
>> BITMAPINFOHEADER */
>> + BITMAPINFOHEADER*header = (BITMAPINFOHEADER*)*data;
>>
>> + ULONG count = header->biClrUsed;
>>
>> + else if (header_size == sizeof(BITMAPCOREHEADER))
>> + {
>> + BITMAPCOREHEADER*header = (BITMAPCOREHEADER*)*data;
>>
>
> I don't think it's a good idea to use variable names multiple times. While
> this works fine it may be a bit confusing...
>
>
I don't find it confusing but I don't mind changing it.
