Eli Zaretskii wrote: >> Date: Fri, 16 Mar 2007 15:03:21 +0000 >> From: "pedro alves" >> Cc: gdb-patches@sourceware.org >> >> >> I'm using the same api as strerror, which returns a pointer into a >> static buffer. >> > > In that case, I guess it's okay to use a static buffer. But please at > least check inside the function that the buffer size was enough, by > comparing the value returned by FormatMessageW with the size of the > buffer you passed to it. > I had thought that the message would be truncated, so I didn't care about that check, but that's not what happens. If the buffer isn't enough, 0 is returned. Ended up switching to FORMAT_MESSAGE_ALLOCATE_BUFFER. > >> Hummm, I don't know how is it that I specified a buffer of size 0, >> and Windows (at least CE) still fills the buffer. >> > > I think it fills the buffer because the buffer is larger than 0. The > return value tells you how many characters it _really_ needed to > format the message. By comparing that value with the size of the > buffer, you can find out whether the buffer was large enough. > > I have no idea how I missed it, but it doesn't fill the buffer after all. >>>> +#ifdef __MINGW32CE__ >>>> + err = strwinerror (GetLastError ()); >>>> +#else >>>> err = strerror (errno); >>>> +#endif >>>> >>> Why not call strwinerror strerror and avoid the ifdef? >>> >> Because then I would have to: >> >> #ifdef __MINGW32CE__ >> #define errno (GetLastError ()) >> #endif >> >> err = strerror (errno); >> >> That means I still must have an #ifdef somewhere. >> > > But that ifdef would be in only one place, while with a different > function you need an ifdef each time you use the function. > > >> Since there is only one instance of it (the #ifdef) in gdbserver, I >> thought it is better to have strwinerror explicit. >> > > There's only one instance _today_. Tomorrow we could have more of > them. > > OK. Moved strwinerror to win32-low.c, and defined strerror to strwinerror in wincecompat.h, which is then included in server.h. > Btw, I see in your patch two instances of strwinerror and two places > that call it: one in gdbreplay.c, the other in utils.c. Why did you > need two almost identical functions? > > Well, gdbreplay (a separate application) has some other functionality that is copied from gdbserver instead of sharing objects, eg: perror_with_name. I didn't think it was worth it to change how it is built for just one function. They are not identical, because the gdbserver version only cares about UNICODE (Win32 API on Windows CE is only wide). >> When someone later uses errno as an lvalue, it breaks WinCE again. >> > > I think such usage of errno is a bad idea anyway, since on many > platforms errno is a function already (to support multi-threading). > > As Andreas said, it must still be an lvalue. The usual way to get around it is to use something like __set_errno(VALUE) instead of errno = VALUE, and define __set_errno to SetLastError on WinCE. >> The other place I used strwinerror is in: >> >> if (!ret) >> { >> - error ("Error creating process %s, (error %d): %s\n", args, >> - (int) GetLastError (), strerror (GetLastError ())); >> + error ("Error creating process \"%s%s\", (error %d): %s\n", >> + program, args, >> + (int) GetLastError (), strwinerror (GetLastError ())); >> } >> >> On Window 9x/NT it is wrong to do: >> strerror (GetLastError ()) >> >> The errno values and the windows error codes are not the same. >> > > This is not a problem: errno should not be used with any literal > values anyway, only with symbolical constants. > > I don't understand what you mean here. On MinGW, this: printf ("%s\n", strerror (ERROR_TOO_MANY_OPEN_FILES)); prints: "Interrupted function call" Clearly not what was intended. This happens because: errno.h #define EINTR 4 /* Interrupted function call */ winerror.h #define ERROR_TOO_MANY_OPEN_FILES 4L >> Would you prefer to have that? That is, rewrite this last hunk to use >> FormatMessage >> directly, and rename strwinerror to strerror, put it wincecompat.c, and have a: >> >> #ifdef __MINGW32CE__ >> #define errno (GetLastError ()) >> #endif >> > > Yes, I'd prefer that solution. gdbserver/win32-low.c is already > Windows specific, so it's okay to use FormatMessage there. > Done. See new attached patch. > Are there any WinCE-specific commands or features left after your > patch? If there are, please describe them in this section. If not, > then it's okay to delete the section. > > There aren't any left.