Mike McCormack wrote:
+#ifdef __WINESRC__ +/* This file contains macros that cause warnings on gcc 4.1, so avoid it. */ +#error Please avoid use of windowsx.h in Wine source code. +#endif
Why don't we fix the macros as suggested by Francois, rather than preventing its use? It seems like a big waste of time to have submitted all of the patches to the files using windowsx.h when fixing the macros is necessary for Winelib anyway.
Robert Shearman wrote:
Mike McCormack wrote:
+#ifdef __WINESRC__ +/* This file contains macros that cause warnings on gcc 4.1, so avoid it. */ +#error Please avoid use of windowsx.h in Wine source code. +#endif
Why don't we fix the macros as suggested by Francois, rather than preventing its use? It seems like a big waste of time to have submitted all of the patches to the files using windowsx.h when fixing the macros is necessary for Winelib anyway.
There is one more solution that wasn't mentioned here. How about using a form like: (void)ListView_GetItem(...); It's a fix on the caller side, but it's really a minimal change and it could be even done by a script.
BTW I agree that code like that looks ugly.
Jacek
Robert Shearman wrote:
Why don't we fix the macros as suggested by Francois, rather than preventing its use? It seems like a big waste of time to have submitted all of the patches to the files using windowsx.h when fixing the macros is necessary for Winelib anyway.
No, it's not a waste of time. Having warning free Wine source code is far more important than any warnings that might be in 3rd party winelib code.
Francois' solution is still not perfect, as it requires use of a non-portable C construct.
Mike
Mike McCormack wrote:
Robert Shearman wrote:
Why don't we fix the macros as suggested by Francois, rather than preventing its use? It seems like a big waste of time to have submitted all of the patches to the files using windowsx.h when fixing the macros is necessary for Winelib anyway.
No, it's not a waste of time. Having warning free Wine source code is far more important than any warnings that might be in 3rd party winelib code.
Francois' solution is still not perfect, as it requires use of a non-portable C construct.
We're trying to work around GCC warnings that probably aren't generated using any other compiler so using non-portable C constructs is perfectly fine. Here is Francois' solution again:
+#ifndef WINE_CAST +# if __GNUC__ +# define WINE_CAST(ctype, val) ({ctype r=(ctype)(val);r;}) +# else +# define WINE_CAST(ctype, val) ((ctype)(val)) +# endif +#endif
- #define Header_SetImageList(hwnd,himl) \
- (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl)
- WINE_CAST(HIMAGELIST,
SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl))
It seemed to be ignored by both you and Alexandre last time it was sent, and all of your patches applied anyway despite objections from several developers.
Robert Shearman wrote:
We're trying to work around GCC warnings that probably aren't generated using any other compiler so using non-portable C constructs is perfectly fine. Here is Francois' solution again:
I understand Francois's solution, as I mentioned that the gcc guys proposed it in the first place. Sorry, but I don't think using non-portable C constructs to solve a warning is a good solution. Why layer on complexity when there's a simple solution?
It seemed to be ignored by both you and Alexandre last time it was sent, and all of your patches applied anyway despite objections from several developers.
If you don't like it, show me your patches to fix the problem.
Mike
Robert Shearman rob@codeweavers.com writes:
It seemed to be ignored by both you and Alexandre last time it was sent, and all of your patches applied anyway despite objections from several developers.
I think these windowsx macros are confusing things more than helping, especially since they don't look like macros. It's better to have explicit SendMessage calls than some pseudo function calls, it makes much more obvious what's going on, especially for people who try to debug the code without being very familiar with the Windows API. The fact that getting rid of the macros also avoids the warnings is just icing on the cake.
2006/3/19, Alexandre Julliard julliard@winehq.org:
I think these windowsx macros are confusing things more than helping, especially since they don't look like macros. It's better to have explicit SendMessage calls than some pseudo function calls, it makes much more obvious what's going on, especially for people who try to debug the code without being very familiar with the Windows API. The fact that getting rid of the macros also avoids the warnings is just icing on the cake.
Just one random example out of the committed Winefile patch:
- int idx = ListBox_AddString(hlbox, infoStr); - ListBox_SetItemData(hlbox, idx, pTxt); + int idx = SendMessage(hlbox, LB_ADDSTRING, 0L, (LPARAM)infoStr); + SendMessage(hlbox, LB_SETITEMDATA, idx, (LPARAM) pTxt);
You can't tell us those SendMessage calls are cleaner and more easy to understand. They contain those ugly (LPARAM) type casts, unused "0L" wparam parameters and result in longer, less readable code.
Regards,
Martin
"Martin Fuchs" martin-fuchs@gmx.net writes:
Just one random example out of the committed Winefile patch:
- int idx = ListBox_AddString(hlbox, infoStr);
- ListBox_SetItemData(hlbox, idx, pTxt);
- int idx = SendMessage(hlbox, LB_ADDSTRING, 0L, (LPARAM)infoStr);
- SendMessage(hlbox, LB_SETITEMDATA, idx, (LPARAM) pTxt);
You can't tell us those SendMessage calls are cleaner and more easy to understand. They contain those ugly (LPARAM) type casts, unused "0L" wparam parameters and result in longer, less readable code.
Not necessarily; the code makes it obvious that we are sending a message here, not calling a function (or worse, a macro disguised as a function). In many cases that's important information, for instance when you try to match a message trace with the code. The casts are ugly but they are present in the macro too, so it isn't any more typesafe, it just looks like it is.
-
Alexandre Julliard -
Jacek Caban -
Martin Fuchs -
Mike McCormack -
Robert Shearman