Re: [Boost-build] Milestone 11, release candidate 3

From: Vladimir Prus (ghost_at_[hidden])
Date: 2006-07-15 01:59:20

On Friday 14 July 2006 00:55, Bronek Kozicki wrote:

Hi Bronek,

> Vladimir Prus wrote:
> > While we're at it, can be have a comment for this function, at least
> > saying what are 'parent' and 'child' arguments. Are they pids or
> > something else?
>
> these are process IDs. I will supply some comment there.

Thanks.

> > This will be clearer if written like:
> >
> >                 if (stricmp(pinfo.szExeFile, "explorer.exe") == 0)
>
> OK.
>
> > The last line of this comment is longer than 80 characters. At the same
> > time, thanks for detailed comments! This is tricky code so it's good to
> > have explanations.
>
> ok, but I think that instead of reformatting I will implement workaround
> suggested by Chris Kohlhoff. With comments, too ;)

You choose ;-)

> >> +        PostMessageA(hwnd, WM_CLOSE, 0, 0);
> >> +        /* now wait and see if it worked. If not, insist */
> >> +        if (WaitForSingleObject(p.h, 200) == WAIT_TIMEOUT)
> >> +        {
> >> +            PostThreadMessageA(tid, WM_QUIT, 0, 0);
> >> +            if (WaitForSingleObject(p.h, 500) == WAIT_TIMEOUT)
> >> +            {
> >> +                PostThreadMessageA(tid, WM_QUIT, 0, 0);
> >> +                WaitForSingleObject(p.h, 500);
> >
> > Is posting WM_QUIT message twice better than posting it once?
>
> this is why I call it "insisting". Windows messages are sometimes a little
> unreliable (IMHO) and if first message "misses" there is small chance than
> next one will "hit" . Maybe it's needless, but it's also harmless.

Maybe you can say that messages are a bit unreliable right in comments. 
Non-Win32 person like me might think they are always delivered.

> > Is the point here to close dialogs right away, and not wait possibly long
> > time for timeout? If so, maybe this should be added to comment?
>
> exactly. The code to automatically close message boxes is quite useless if
> it does not close the window in reasonably short time - the whole purpose
> of this code is to let us (testers) use long process timeouts (option -l ,
> so that slow compilers or tests have time to do their job) without
> tremendously slowing tests while displaying these messages. It's especially
> useful for all kinds of experiments, when these dialogs may appear quite
> often.

Ok, I see.

> > Overall, looks reasonable. Would you like to address the comments and
> > sent the patch again, or it will be more convenient if I commit the patch
> > right now and you'll send further patches when you have the time?
>
> submit now, because the code is well tested and works, and I will try to
> prepare another patch before weekend.

Okay, the code in in CVS now. I've also merged to to release branch, since the 
code is likely to be of help when testing.

Thanks, and looking for next patch!

- Volodya

-- 
Vladimir Prus
http://vladimir_prus.blogspot.com
Boost.Build V2: http://boost.org/boost-build2