From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31469 invoked by alias); 30 Mar 2011 21:34:25 -0000 Received: (qmail 31114 invoked by uid 22791); 30 Mar 2011 21:34:19 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_MULTIPLE_AT,TW_CP X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.153) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 30 Mar 2011 21:32:54 +0000 Received: from md1.u-strasbg.fr (md1.u-strasbg.fr [IPv6:2001:660:2402::186]) by mailhost.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id p2ULWmRr031673 ; Wed, 30 Mar 2011 23:32:48 +0200 (CEST) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from mailserver.u-strasbg.fr (ms2.u-strasbg.fr [130.79.204.11]) by md1.u-strasbg.fr (8.14.4/jtpda-5.5pre1) with ESMTP id p2ULWl51082881 ; Wed, 30 Mar 2011 23:32:48 +0200 (CEST) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (user=mullerp mech=LOGIN) by mailserver.u-strasbg.fr (8.14.4/jtpda-5.5pre1) with ESMTP id p2ULWfNm017625 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO) ; Wed, 30 Mar 2011 23:32:47 +0200 (CEST) (envelope-from pierre.muller@ics-cnrs.unistra.fr) From: "Pierre Muller" To: "'Eli Zaretskii'" Cc: References: <00a201cbeed2$a924dfa0$fb6e9ee0$%muller@ics-cnrs.unistra.fr> <83lizwqtz9.fsf@gnu.org> In-Reply-To: <83lizwqtz9.fsf@gnu.org> Subject: RE: [RFC 1/9] Unify windows specifics into common/windows-hdep files Date: Thu, 31 Mar 2011 11:23:00 -0000 Message-ID: <003f01cbef22$038bef20$0aa3cd60$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg01211.txt.bz2 > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Eli Zaretskii > Envoy=E9=A0: mercredi 30 mars 2011 21:39 > =C0=A0: Pierre Muller > Cc=A0: gdb-patches@sourceware.org > Objet=A0: Re: [RFC 1/9] Unify windows specifics into common/windows-hdep files >=20 > > From: "Pierre Muller" > > Date: Wed, 30 Mar 2011 14:04:45 +0200 > > > > This unifies windows path handling routines and > > macros related to use of ASCII or WIDE version of Windows API. > ^^^^^ > You mean "ANSI", I presume. Yes you are right, I should use the Microsoft API wording of ANSI versus Unicode. =20 > > +/* The primary purpose of this file is to implement conversion between > > + POSIX style and Windows NATIVE style path and path lists. >=20 > GNU coding standards frown on using "path" for anything but PATH-style > lists of directories. You should use "file name" instead. I have no opinion on that subject, I just wanted to stay close to the cygwin function names... =20 > Same comment on the "path" part in the name of the functions you > coded. >=20 > > +int > > +windows_conv_path (conv_type oper, const void *from, void *to, int size) > > +{ > > +#ifdef USE_MINGW_CONV > > + if (size =3D=3D 0) > > + { > > +#ifdef USE_WIDE_WINAPI > > + if (oper =3D=3D WINDOWS_POSIX_TO_NATIVE_W) > > + return MultiByteToWideChar (CP_ACP, 0, from, -1, to, 0); > > + if (oper =3D=3D WINDOWS_NATIVE_W_TO_POSIX) > > + return WideCharToMultiByte (CP_ACP, 0, from, -1, to, 0, 0, 0); > > + else > > +#endif /* USE_WIDE_WINAPI */ >=20 > Hmm... I'm not sure I understand the intent. Conversion from UTF-16 > to the ANSI codepage is by definition lossy; what do you expect to > come out of this conversion in those cases? And you don't even use > the WC_NO_BEST_FIT_CHARS flag (as MSDN says you should when converting > file names), nor check for errors with GetLastError. It looks like > this can never work reliably, or am I missing something? I didn't check those options. I will need to read their descriptions=20 more precisely. =20 > > + int len =3D strlen((const char *) from) + 1; > > +#ifdef USE_WIDE_WINAPI > > + if (oper =3D=3D WINDOWS_POSIX_TO_NATIVE_W) > > + return MultiByteToWideChar (CP_ACP, 0, from, len, to, size); > > + if (oper =3D=3D WINDOWS_NATIVE_W_TO_POSIX) > > + return WideCharToMultiByte (CP_ACP, 0, from, len, to, size, 0, 0); > > + else > > +#endif /* USE_WIDE_WINAPI */ > > + { > > + if (size < len) > > + len =3D size; > > + strncpy ((char *) to, (const char *) from, len); > > + if (oper =3D=3D WINDOWS_NATIVE_TO_MSYS) > > + { > > + char * p; > > + p =3D strchr (to, '\\'); > > + while (p) > > + { > > + *p =3D '/'; > > + p =3D strchr (to, '\\'); > > + } >=20 > Here, I don't understand why you flip the slashes only when `oper' is > something other that WINDOWS_POSIX_TO_NATIVE_W or > WINDOWS_NATIVE_W_TO_POSIX. Why not flip the slashes together with > converting from multibyte to wide characters and back? I agree that I also should do the flips on the two above cases... =20 > > + if (((char *) to)[1] =3D=3D ':' && ((char *) to)[2] =3D=3D '/') >=20 > Is it guaranteed that `to' is at least 2 characters long? What if > it's only 1 character long? One more missing check, thanks for noticing.=20 =20 > > + return cygwin_conv_to_full_posix_path (from, to); > > + break; >=20 > Why do we need a `break' after a `return'? Missing C code practice :(=20 > > +/* Analogeous function for PATH style lists. */ > ^^^^^^^^^^ > A typo. Apparently, at least I was consistent ... =20 > > + GDB with wide char use even on systems for which the default is to > > + use ASCII chars. */ > ^^^^^ > "ANSI". >=20 > > +#undef _G > > +/* Macro _G_SUFFIX(text) expands either to 'text "A"' or to 'text "W"' > > + depending on the use of wide chars or not. */ > > +#undef _G_SUFFIX >=20 > I think the C Standard says that macros whose name begins with an > underscore and a capital letter are reserved. Applications should not > use such macros. But we are also using __USEWIDE before my patch ... or do you mean that two underscores are OK? > > + Otherwise mingw compiler is assumed, which defaults to use of ascii > > + WINAPI for now (this may change later). ^^^^^ >=20 > "ANSI". >=20 > > + Default can be overridden either by defining USE_WIDE_WINAPI > > + to force use of wide API, or by defining USE_ASCII_WINAPI to prevent > > + use of wide API. */ >=20 > But do we actually support that? AFAIK, to get the wide APIs, you > need to compile with _UNICODE defined. But we don't have such > definitions anywhere, do we? I completely remove __USEWIDE and tried to replace it by this new USE_WIDE_WINAPI. Or is this __USEWIDE macro used anywhere else than inside GDB code? Should I rather call the macro USE_UNICODE_WINAPI (and USE_ANSI_WINAPI to force ANSI version?)=20 =20 > > +# define CreateProcess CreateProcessW > > +# define GetSystemDirectory GetSystemDirectoryW > > +# define windows_strlen wcslen >=20 > Ouch! So any API that needs one of the two varieties will need to be > added to this list of #define's? Is that wise? Isn't it better than being forced to use #ifdef __USEWIDE CreateProcessW (... #else CreateProcessA (... #endif =20 > > +typedef enum > > +{ > > + WINDOWS_NATIVE_A_TO_POSIX =3D 1 > > + ,WINDOWS_POSIX_TO_NATIVE_A =3D 2 > > +#ifdef USE_WIDE_WINAPI > > + ,WINDOWS_NATIVE_W_TO_POSIX =3D 3 > > + ,WINDOWS_POSIX_TO_NATIVE_W =3D 4 > > +#endif > > + ,WINDOWS_NATIVE_TO_MSYS =3D 5 >=20 > I think this is ugly. Why cannot you put the commas after the values? No reason, it's just that I also find it ugly to have a comma after the last enum (but I am not even forced to do that here, so my argument is bad...) =20 > > + Returns size of converted string in characters or > > + -1 if there was an error. */ > > + > > +extern int windows_conv_path (conv_type oper, const void *from, > > + void *to, int size); >=20 > The return value is not -1 when it fails, it's zero, because that's > what MultiByteToWideChar and WideCharToMultiByte return in that case. Here you are talking about the Mingw version: I agree that I must check failure of those calls and return -1 in that case. =20 > > +/* Analogeous function for PATH style lists. */ > ^^^^^^^^^^ > A typo. Agreed Thanks for the useful comments. Pierre