File: dlls\comdlg32\printdlg.c
Function: static void subclass_margin_edits(HWND hDlg)

Hi WineDevs,
I have a revision for the common page setup dialog for You.

I'm not a Wine dev, but a participant in ReactOS, and we synchronize
ComDlg32 with You regularly, so this revision _should_ go to You.
I'll submit the full function, not a patch, since I don't work in
the Wine source tree. The ReactOS sync has a different path, and
most likely line numbers too, so a patch merge would go astray.
I've registered with the wine-patches and wine-devel mailing lists
so You can notify in case You need something else from me.

Since this is not a straight 'diff' mail-body I'll post it here on
wine-devel first, as I'm unsure how You handle wine-patches mail.

Now to the small revision:

This revision adds an error check on the winproc pointer returned
by SetWindowLongPtr(.. GWLP_WNDPROC ..) and fixes a build warning
issued because GCC thinks 'old_proc' is computed but never used..

I assume the reason for repeating the attempt to set 'edit_proc' is
because we're not certain which editors will be present, so there
_should_ be a fail-check on 'old_proc' anyway.

If you _know_ 'edt4' will always be present, it would be better to
move the assign for 'edit_proc' out of the loop, and avoid the
repeated conditional assign.  Both variants are #if'ed in the patch.
Just remove the variant you don't want.

--- code begin ---
static void subclass_margin_edits(HWND hDlg)
{
   int id;
   WNDPROC old_proc;
#if 1
   for(id = edt4; id <= edt7; id++)
   {
       old_proc = (WNDPROC)SetWindowLongPtrW(GetDlgItem(hDlg, id),
                                             GWLP_WNDPROC,
(ULONG_PTR)pagesetup_margin_editproc); /* InterlockedCompareExchangePointer((void**)&edit_wndproc, old_proc, NULL); */
       if ((edit_wndproc == NULL) && (old_proc != NULL))
           edit_wndproc = old_proc;
   }
#else
   old_proc = (WNDPROC)
       SetWindowLongPtrW (
           GetDlgItem( hDlg, edt4 ),
           GWLP_WNDPROC, (ULONG_PTR) pagesetup_margin_editproc
           );
   if ((edit_wndproc == NULL) && (old_proc != NULL))
       edit_wndproc = old_proc;

   for( id = edt5; id <= edt7; id++ )
       SetWindowLongPtrW (
           GetDlgItem( hDlg, id ),
           GWLP_WNDPROC, (ULONG_PTR) pagesetup_margin_editproc
           );
#endif
}
--- code end ---

The reason for the build warning is that the definition
of InterlockedCompareExchangePointer() confuses GCC.
It thinks 'old_proc' is computed but never used,
and issues an over-sensitive, even misleading, warning.
It seems like GCC doesn't realize that an intrinsic uses the value.
It would be good to fix InterlockedCompareExchangePointer, or GCC.
Sadly, I can't figure out how to do that.

Best Regards
 Love N



Reply via email to