Re: [Wireshark-commits] rev 45212: /trunk/ui/gtk/ /trunk/ui/gtk/: gui_utils.c

["Maynard, Chris" <Christopher.Maynard () GTECH COM>]

Good point Bill.  I hadn't actually looked too deeply here; I just wanted to appease the buildbot.  See: 
http://buildbot.wireshark.org/trunk/builders/Windows-XP-x86/builds/2646/steps/nmake%20all/logs/stdio

So maybe a minimal patch such as this is what is needed?  Maybe that was the original intent?
- Chris

Index: ui/gtk/gui_utils.c
===================================================================
--- ui/gtk/gui_utils.c  (revision 45212)
+++ ui/gtk/gui_utils.c  (working copy)
@@ -694,7 +694,7 @@
 {
     HANDLE        handle;
     DWORD         avail      = 0;
-    gboolean      result;
+    gboolean      result, result1;
     DWORD         childstatus;
     pipe_input_t *pipe_input = data;
     gint          iterations = 0;
@@ -710,12 +710,13 @@
         result = PeekNamedPipe(handle, NULL, 0, NULL, &avail, NULL);
         /* Get the child process exit status */
-       GetExitCodeProcess((HANDLE)*(pipe_input->child_process), &childstatus);
+        result1 = GetExitCodeProcess((HANDLE)*(pipe_input->child_process),
+                                     &childstatus);
         /* If the Peek returned an error, or there are bytes to be read
            or the childwatcher thread has terminated then call the normal
            callback */
-        if (!result || avail > 0 || childstatus != STILL_ACTIVE) {
+        if (!result || avail > 0 || result1 || childstatus != STILL_ACTIVE) {
             /*g_log(NULL, G_LOG_LEVEL_DEBUG, "pipe_timer_cb: data avail");*/


- Chris

________________________________________
From: wireshark-dev-bounces () wireshark org [wireshark-dev-bounces () wireshark org] On Behalf Of Bill Meier [wmeier 
() newsguy com]
Sent: Saturday, September 29, 2012 12:13 PM
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 45212: /trunk/ui/gtk/ /trunk/ui/gtk/: gui_utils.c

On 9/29/2012 12:02 PM, Bill Meier wrote:
> On 9/29/2012 11:49 AM, cmaynard () wireshark org wrote:
> > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=45212
> >
> > User: cmaynard
> > Date: 2012/09/29 08:49 AM
> >
> > Log:
> >   Revert most of r45210. We don't use the return value of
> > GetExitCodeProcess(), but it still needs to be called to get childstatus.
>
> Looking at
> http://msdn.microsoft.com/en-us/library/windows/desktop/ms683189%28v=vs.85%29.aspx
>   [1]
>
> Two comments:
>
> 1. I suspect that testing 'childstatus' may not be valid if
> getExitCodeprocess returns FALSE;
>
> 2. I don't understand the 'Important' para in the 'Remarks' section.
>
> The first para under 'Remarks' and the 'Important' para seem to
> contradict each other (altho the first para uses 'process' and the
> 'Important' para uses 'thread').
>
>
> Not being a Windows API programmer, I'll leave this to others ....
>
> Bill
>
>
> [1]
> Remarks
>
> This function returns immediately. If the process has not terminated and
> the function succeeds, the status returned is STILL_ACTIVE. If the
> process has terminated and the function succeeds, the status returned is
> one of the following values:
>
>      The exit value specified in the ExitProcess or TerminateProcess
> function.
>      The return value from the main or WinMain function of the process.
>      The exception value for an unhandled exception that caused the
> process to terminate.
>
> Important  The GetExitCodeProcess function returns a valid error code
> defined by the application only after the thread terminates. Therefore,
> an application should not use STILL_ACTIVE (259) as an error code. If a
> thread returns STILL_ACTIVE (259) as an error code, applications that
> test for this value could interpret it to mean that the thread is still
> running and continue to test for the completion of the thread after the
> thread has terminated, which could put the application into an infinite
> loop.


Now I get it.

... an *application* should not use ...

So: maybe the only issue is whether the return TRUE/FALSE value can be
ignored (as the code has obviously been doing for quite some time).




___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
CONFIDENTIALITY NOTICE: The information contained in this email message is intended only for use of the intended 
recipient. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, 
distribution or copying of this communication is strictly prohibited. If you have received this communication in error, 
please immediately delete it from your system and notify the sender by replying to this email.  Thank you.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe