Re: Make RECT === RECTL, fix compile warnings, use uniform format

[Dimitrie O. Paun]

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.

Make RECT === RECTL, fix compile warnings, use uniform format

[Dan Kegel]

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...))

Make RECT === RECTL, fix compile warnings, use uniform format

[Dimitrie O. Paun]

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?

Make RECT === RECTL, fix compile warnings, use uniform format

[Dan Kegel]

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

Make RECT === RECTL, fix compile warnings, use uniform format

[Dimitrie O. Paun]

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! :)

Make RECT === RECTL, fix compile warnings, use uniform format

[Alexandre Julliard]

"Dimitrie O. Paun"  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.

Make RECT === RECTL, fix compile warnings, use uniform format

[Dimitrie O. Paun]

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?

Make RECT === RECTL, fix compile warnings, use uniform format

[Alexandre Julliard]

"Dimitrie O. Paun"  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.