incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [patch] exec utility windows port
Date Fri, 18 Aug 2006 18:09:22 GMT
Andrew Black wrote:
> I have a couple thoughts about this proposed patch.
> 
> The first observation is related to the warn_last_error change.  It 
> appears to me that the type of error_text should be changed from LPTSTR 
> to TCHAR, eliminating the need for the typecast in the call to 
> FormatMessage.

TCHAR is the same thing as char and LPTSTR is the same thing as char*
as far as we're concerned. The names are Microsoft's harebrained idea
for an internationalized API. When FormatMessage is told to allocate
the string, the char* lpBuffer argument is expected to be a pointer
to void*, i.e., void**, and the function does something like this:

     *(void**)lpBuffer = LocalAlloc (..., nSize);
     sprintf (*(char**)lpBuffer, "%s", message);

There is no clean way for the caller to pass a void** to a function
that takes a char*. Clearly a design flaw in the API but what else
is new. We will have to live with the cast.

> Also, the return value from LocalFree probably should be 
> checked (There isn't much we can do if it fails, but we probably should 
> warn the user that it failed, and provide him/her with the error number, 
> but not a string translation to avoid entering a potentially infinite 
> loop).

Right. We should definitely check the return values of most calls
and report errors or warnings. But trying to handle errors that
occur while handling other errors is a catch 22, so I would skip
this type of error checking and reporting in warn() or terminate()
(although if FormatMessage fails we should still try to produce
an error message without the formatted text).

> 
> The second observation is related to the final call to 
> WaitForSingleObject and the new calls to CloseHandle.  For consistency, 
> we should check the return value from each of these calls, and call 
> warn_last_error if they failed (failure being defined as 0 for 
> CloseHandle and and WAIT_FAILED for WaitForSingleObject).

I agree. Can you post a patch?

Martin

Mime
View raw message