2013/4/23 Rico Sch�ller <kgbricola@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.