Re: Make RECT === RECTL, fix compile warnings, use uniform format
On December 28, 2002 04:10 pm, Dan Kegel wrote:
Decided to leave debugrect for later, too much of a pain for now. This patch just uses a uniform format for RECTs where possible. It reorders three or so TRACEs that had the coordinates in nonstandard order, too. Patch against current CVS attached. Should compile fine, but I haven't tested it (so do look it over :-)
Cool stuff. Can you please turn this thing: Index: dlls/comctl32/datetime.c =================================================================== RCS file: /home/wine/wine/dlls/comctl32/datetime.c,v retrieving revision 1.34 diff -d -u -r1.34 datetime.c --- dlls/comctl32/datetime.c 2 Dec 2002 18:11:00 -0000 1.34 +++ dlls/comctl32/datetime.c 28 Dec 2002 20:43:22 -0000 @@ -1098,7 +1098,7 @@ infoPtr->rcClient.bottom = HIWORD(lParam); infoPtr->rcClient.right = LOWORD(lParam); - TRACE("Height=%d, Width=%d\n", infoPtr->rcClient.bottom, infoPtr->rcClient.right); + TRACE("Height=%ld, Width=%ld\n", infoPtr->rcClient.bottom, infoPtr->rcClient.right); /* use DrawEdge to adjust the size of rcEdge to get rcDraw */ memcpy((&infoPtr->rcDraw), (&infoPtr->rcClient), sizeof(infoPtr->rcDraw)); Into a standard [(%ld,%ld)-(%ld,%ld)]? As for debugrect(), it seems like half of the printed rectangles are in commctrl, so what we can do is use debugrect() in there without having to introduce any infrastructure changes. We can do that after Alexandre commits this change, so we don't cram too much stuff into one patch. -- Dimi.
Dimitrie O. Paun wrote:
On December 28, 2002 04:10 pm, Dan Kegel wrote:
Decided to leave debugrect for later, too much of a pain for now. This patch just uses a uniform format for RECTs where possible. It reorders three or so TRACEs that had the coordinates in nonstandard order, too. Patch against current CVS attached. Should compile fine, but I haven't tested it (so do look it over :-)
Cool stuff. Can you please turn this thing:
Index: dlls/comctl32/datetime.c =================================================================== RCS file: /home/wine/wine/dlls/comctl32/datetime.c,v retrieving revision 1.34 diff -d -u -r1.34 datetime.c --- dlls/comctl32/datetime.c 2 Dec 2002 18:11:00 -0000 1.34 +++ dlls/comctl32/datetime.c 28 Dec 2002 20:43:22 -0000 @@ -1098,7 +1098,7 @@ infoPtr->rcClient.bottom = HIWORD(lParam); infoPtr->rcClient.right = LOWORD(lParam);
- TRACE("Height=%d, Width=%d\n", infoPtr->rcClient.bottom, infoPtr->rcClient.right); + TRACE("Height=%ld, Width=%ld\n", infoPtr->rcClient.bottom, infoPtr->rcClient.right);
/* use DrawEdge to adjust the size of rcEdge to get rcDraw */ memcpy((&infoPtr->rcDraw), (&infoPtr->rcClient), sizeof(infoPtr->rcDraw));
Into a standard [(%ld,%ld)-(%ld,%ld)]?
Aw, let's leave that for when we do the debugrect() conversion. Or you can do it if you like. I think I'm done with RECT changes for a while. I'm trying to fix ShellExecute now, seems higher priority...
As for debugrect(), it seems like half of the printed rectangles are in commctrl, so what we can do is use debugrect() in there without having to introduce any infrastructure changes. We can do that after Alexandre commits this change, so we don't cram too much stuff into one patch.
OK. Yes, let's keep this first patch simple. (BTW it might worth looking into a change to debugrect() to get its storage on the stack, in a local variable of the TRACE() macro. That'd avoid possible thread clashes (if you have too many threads, the current way debugrect() gets storage might break...)) -- Dan Kegel Linux User #78045 http://www.kegel.com
On December 29, 2002 04:10 pm, Dan Kegel wrote:
(BTW it might worth looking into a change to debugrect() to get its storage on the stack, in a local variable of the TRACE() macro. That'd avoid possible thread clashes (if you have too many threads, the current way debugrect() gets storage might break...))
Yes, in theory the current implementation has that problem, but in practice is OK. And besides, it's for debugging only. But how can you get that local var without changing the current interface? -- Dimi.
Dimitrie O. Paun wrote:
On December 29, 2002 04:10 pm, Dan Kegel wrote:
(BTW it might worth looking into a change to debugrect() to get its storage on the stack, in a local variable of the TRACE() macro. That'd avoid possible thread clashes (if you have too many threads, the current way debugrect() gets storage might break...))
Yes, in theory the current implementation has that problem, but in practice is OK. And besides, it's for debugging only. But how can you get that local var without changing the current interface?
You could do something cheezy like --- debug.h 2002-07-05 14:22:09.000000000 -0700 +++ debug.h.new 2002-12-31 00:01:33.000000000 -0800 @@ -80,6 +80,8 @@ #define __WINE_DPRINTF(dbcl,dbch) \ do { if(__WINE_GET_DEBUGGING(dbcl,(dbch))) { \ + char __wine_dprintf_buf[2048]; \ + char *__wine_dprintf_ptr = __wine_dprintf_buf; \ const char * const __dbch = (dbch); \ const enum __WINE_DEBUG_CLASS __dbcl = __WINE_DBCL##dbcl; \ __WINE_DBG_LOG @@ -87,6 +89,8 @@ #define __WINE_DBG_LOG(...) \ wine_dbg_log( __dbcl, __dbch, __func__, __VA_ARGS__); } } while(0) +#define debugrect(r) __debugrect((r), &__wine_dprintf_ptr) + #define __WINE_PRINTF_ATTR(fmt,args) #else /* !__GNUC__ && !__SUNPRO_C */ BTW debugrect() isn't a great name... can't think of a snappy replacement offhand, though. - Dan -- Dan Kegel Linux User #78045 http://www.kegel.com
On December 31, 2002 03:03 am, Dan Kegel wrote:
+#define debugrect(r) __debugrect((r), &__wine_dprintf_ptr)
Yeah, that can work too, but it does increase stack usage by quite a bit. But if we are to do this for RECTs only we don't need as much space anyway... Nevertheless, it's an alternative solution, and it does not require exporting more stuff out of ntdll. Let's wait for Alexandre's take on it. :)
BTW debugrect() isn't a great name... can't think of a snappy replacement offhand, though.
True, but it has the following nice caracteristics: -- uses the debug prefix -- easy to remember if we have the naming convention: debug<struct-name-to-lower-case> -- it is short Would be nice to have such std functions for a bunch of structures, like I did in listview.c. They are extremely handy, and can save you quite a bit of time when you want to dump stuff out. And if more of these become standard, a simple naming convention like this one pays. Feel free to suggest something else though! :) -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
Yeah, that can work too, but it does increase stack usage by quite a bit. But if we are to do this for RECTs only we don't need as much space anyway... Nevertheless, it's an alternative solution, and it does not require exporting more stuff out of ntdll. Let's wait for Alexandre's take on it. :)
This should use the per-thread buffer that is already used for the debugstr functions. What we need is to export that functionality through a more generic interface so that we can implement all kinds of debug* functions on top of it. I'll look into that. -- Alexandre Julliard julliard(a)winehq.com
On January 2, 2003 12:41 pm, Alexandre Julliard wrote:
What we need is to export that functionality through a more generic interface so that we can implement all kinds of debug* functions on top of it. I'll look into that.
Excellent -- this is the infrastructure change I was referring to in my emails. This is perfect: if you can export that, I can then look into providing some more debug* functions. Make sure we somehow know the size of the returned buffer. Either by a constant, or providing a minimum length argument: char *debug_getbuffer(int size); If the allocation can not be met, we should just crash the program. This way we can maintain 2-4 sizes (32, 64, 256, 1024), and return the closest one that is larger than size. As for the new debug* functions, most nontrivial ones are not that small. Do you want them defined as static inline in wine/debug.h, or exported from ntdll as the other ones? In fact, why not create a helper DLL (winedebug) that holds all of this, and that can be used on Windows as well, when we port most of our stuff over? -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
As for the new debug* functions, most nontrivial ones are not that small. Do you want them defined as static inline in wine/debug.h, or exported from ntdll as the other ones? In fact, why not create a helper DLL (winedebug) that holds all of this, and that can be used on Windows as well, when we port most of our stuff over?
IMO this would be a mechanism to use only for trivial functions that can be put inline; more complex ones should be implemented in their respective dlls. We definitely don't want a big centralized module that dumps all known structures. -- Alexandre Julliard julliard(a)winehq.com
participants (3)
-
Alexandre Julliard -
Dan Kegel -
Dimitrie O. Paun