Question from new developer

Bruno Jesus 00cpxxx at gmail.com
Sun Sep 23 06:08:36 CDT 2012


On Fri, Sep 21, 2012 at 8:41 PM, Chris Teague <chris.teague at gmail.com> wrote:
> I submitted bug 31753 for an application that I use, and started
> making an attempt to fix it.  I started with the last few messages
> from the log:
>
> fixme:win:LockWindowUpdate (0x501c4), partial stub!
> fixme:win:LockWindowUpdate ((nil)), partial stub!
> wine: Unhandled page fault on read access to 0x00000023 at address
> 0x78680087 (thread 0009), starting debugger...
>
> Looking at LockWindowUpdate(), I discovered that it had no conformance
> tests.  I wrote a few covering all the possible scenarios I could
> think of (only 4 of them), and ran them on wine as well as cross
> compiling them and running on XP.  To my surprise, one of the tests
> fails on wine, but passes on WinXP!  This was great news, so I dug a
> little further.  Specifically the problem was attempting to do a
> "unlock" operation by passing NULL, when the the function was already
> unlocked.  Windows treats this as an error and returns 0, while wine
> returns 1.
> So I fixed it, patch is attached (also at:
> https://dl.dropbox.com/u/477050/0001-Fixed-LockWindowUpdate.txt)
> I have two big questions:
> 1. I discovered later that I don't think this is the root of my
> original bug - and in fact doesn't seem to affect behavior of my
> program.  Is it still worth submitting?  I do believe it alleviates
> the need for the "fixme" in the LockWindowUpdate function.

The fixme is there because the function is far away from implemented,
you can read more about it at the long standing bug 52 (
http://bugs.winehq.org/show_bug.cgi?id=52 ).
IMO adding tests that prove something is wrong are always good.

> 2. If it is worth submitting, could someone take a look at it and
> point out all the dumb things I've done?  I'm happy with the logic of
> the code, but I'm sure I've done something horribly wrong in terms of
> doing it the preferred way.

You should join all tests in the same function and use an existing
file. You need to fix the whitespace issues, you didn't follow the
surrounding code style (possibly your editor was configured
differently from the file). You are also not freeing the created
windows.

> Thanks,
> Chris Teague

I'm not used to reviewing patches, I hope someone else can do a better review.

Best wishes,
Bruno



More information about the wine-devel mailing list