A revision for common PageSetup dialog - wine-devel

4 May 2010


      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