[PATCH v3 2/4] include: only define _tcs macros on suitable libc.

[Jacek Caban]

On 01.10.2020 00:30, Puetz Kevin A wrote:
>> -----Original Message-----
>> From: Jacek Caban <jacek at codeweavers.com>
>> Sent: Wednesday, September 30, 2020 4:08 PM
>>
>> Hi Kevin,
>>
>> On 24/09/2020 17:12, Kevin Puetz wrote:
>>> --- a/include/tchar.h
>>> +++ b/include/tchar.h
>>> @@ -37,9 +37,15 @@ extern "C" {
>>>    #define _strninc(str,n)         (((char*)(str))+(n))
>>>    #define _strspnp(s1,s2)         (*((s1)+=strspn((s1),(s2))) ? (s1) : NULL)
>>>
>>> +#if defined(__MSVCRT__) || defined(_MSC_VER) ||
>> (defined(WINE_UNICODE_NATIVE) && defined(_UNICODE)) ||
>> !(defined(_UNICODE) || defined(_MBCS))
>>
>> This seems to be more complicated than it needs to be. Since guarded
>> macros really belong to crt, maybe we should have a guard bellow
>> includes of string.h, wchar.h, etc. (to give corecrt.h a chance to be
>> included) and simply use:
>>
>> #ifdef __WINE_USE_MSVCRT
> I guess I was (again) aiming for as much portability as I could.
>
> Wine 5.0.x allowed the use of tchar.h for 2 cases:
> 1. __MSVCRT__ (i.e. -mno-cygwin)
> 2. "plain" 8-bit text (no _UNICODE or _MBCS)
>
> Anything else got a #error. But any settings allowed to
> #include <tchar.h> at all got the _tcs* functions.
>
> My original patches (which c1208903 applied partially)
> removed that #error to allow using tchar.h with char16_t.
> They kept the _tcs macros guarded since a standard libc
> won't have utf16 functions, but allowed a couple more cases:
>
> 3. _UNICODE && WINE_UNICODE_NATIVE (i.e. -fshort-wchar)
>     This was just by analogy: if char str functions were OK for 8-bit _TCHAR,
>     the wchar_t wcs functions should be equally OK when _TCHAR==wchar_t
> 4. MSVC
>
> I don't really have a specific use for these; I was really submitting this
>   clean-up just because I felt c1208903 had left things inconsistent
> (_tcs macros defined that won't work, or maybe even compile).
> Since that started with (part of) my patch I felt responsible to fix it 😊.
>
> I don't have any objection to simplifying this further, and just ruling that
> _tcs* functions are MSVCRT-only. But I didn't propose that because it
> would be remove some functionality that 5.0.x had.
>
> If you want to go that way, though (make a breaking change from 5.0.x),
> I think it would make more sense to follow-through all the way and match
> the Windows SDK/ucrt split, i.e. move tchar.h into include/msvcrt,
> and leave it with no #ifdef.
>
> TCHAR and TEXT() are already in winnt.h, so it seems defensible to
> give glibc just those, keeping _TCHAR, _TEXT, and _tcs*()
> functions for MSVCRT (where they always have usable definitions).
>
> That would break a few things we're compiling with wineg++,
> but not in ways that would be hard to fix.


I'm not concerned too much about corner case 5.0 compatibility in this 
particular case. I think that realistically you're probably the only 
user of this feature so far, so we have some flexibility for the 
solution. I also think that current behaviour is not too bad: it works 
when it can, otherwise the compiler will issue a warning or an error on 
an attempt to use a problematic macro macro. But if you want to guard 
better, I don't mind, but let's keep it simple so that it's easier to 
read and less likely to require future maintenance.


Thanks,

Jacek