[PATCH 2/8] Files: add wrappers for open(), fopen(), sqlite3_open()

[Lubomir I. Ivanov]

On 19 December 2013 01:13, Thiago Macieira <thiago at macieira.org> wrote:
> Looks good, just one suggestion for improvement.
>
> On quinta-feira, 19 de dezembro de 2013 00:47:00, Lubomir I. Ivanov wrote:
>> --- a/macos.c
>> +++ b/macos.c
>
>> +/* NOP wrappers to comform with windows.c */
>> +int subsurface_open(const char *path, int oflags, mode_t mode)
>> +{
>> +     return open(path, oflags, mode);
>> +}
>> +
>> +FILE *subsurface_fopen(const char *path, const char *mode)
>> +{
>> +     return fopen(path, mode);
>> +}
>> +
>> +int subsurface_sqlite3_open(const char *path, sqlite3 **handle)
>> +{
>> +     return sqlite3_open(path, handle);
>> +}
>
> Technically, on Mac it should be NFD UTF-8, but I guess we should fix that on
> the file name generation side instead. This is a braindead situation because
> applying transformations is likely to cause data loss somewhere. The right
> thing to do would be to change Macs so that their input mechanisms produce NFD
> text in the first place.
>
> I can make some patches for that.
>
>> +/* this function converts a utf-8 string to win32's utf-16 2 byte string.
>> + * the caller function should manage the allocated memory.
>> + */
>> +static wchar_t *utf8_to_utf16_fl(const char *utf8, char *file, int line)
>> +{
>> +     if (!file) {
>> +             file = "unknown";
>> +             line = 0;
>> +     }
>
> Just assert(). We shouldn't pass null pointers here and there's no easy
> recovery anyway.
>

ok, i can make this change, quickly.

>> +     int sz = MultiByteToWideChar(CP_UTF8, 0, utf8, -1, NULL, 0);
>> +     if (!sz) {
>> +             fprintf(stderr, "%s:%d: %s", file, line, "cannot estimate string
> size.");
>> +             return NULL;
>> +     }
>
> UTF-8 to UTF-16 contains always at most the same number of characters. So you
> can replace the call above by a simple strlen(utf8). In the worst case, we'll
> overestimate the buffer by 3x, but since we're going to free it anyway soon
> after, it's no big deal.
>

i saw this used everywhere with 2 consecutive calls, so decided to do
that here as well.
this is usually common practice WINAPI to call the same function with
different arguments and receive different results.

should i replace it to be?:
int sz = strlen(utf) * 3;

>> +FILE *subsurface_fopen(const char *path, const char *mode)
>> +{
>> +     FILE *ret = NULL;
>> +     wchar_t *wpath = utf8_to_utf16(path);
>> +     if (wpath) {
>> +             wchar_t *wmode = utf8_to_utf16(mode);
>
> This one is going to be expensive... the mode is actually US-ASCII, so a
> simpler algorithm would be cheaper, but then we have to write it, maintain it,
> etc. This is fine.
>

yeah, but it still requires a wchar buffer at least in MSVCRT:
http://msdn.microsoft.com/en-us/library/yeby3zcb.aspx
so i left it like that.

lubomir
--