2

In an almost complete effort to port libc++ to Windows, I need the function mbsnrtowcs, and thought that the easiest way would be to implement it in terms of mbsrtowcs:

size_t mbsnrtowcs( wchar_t *__restrict__ dst, const char **__restrict__ src,
               size_t nmc, size_t len, mbstate_t *__restrict__ ps )
{
    char* nmcsrc = new char[nmc+1];
    strncpy( nmcsrc, *src, nmc );
    nmcsrc[nmc] = '\0';
    const size_t result = mbsrtowcs( dst, &nmcsrc, len, ps );
    delete[] nmcsrc;
    return result;
}

The problem here is that mbsrtowcs needs &nmcsrc to be of type const char**, which is isn't, because it is a string I just copied the first nmc elements into, and appended a \0 character to. How can I work around this? Strictly speaking, this is compiled as C++, so perhaps a const_cast would do little harm here? I also have access to c++0X (libc++ is/requires a subset c++0x).

Edit: the Clang error message reads:

M:\Development\Source\libc++\src\support\win32\support.cpp:37:27: error: no matching function for call to 'mbsrtowcs'
const size_t result = mbsrtowcs( dst, &nmcsrc, len, ps );
                      ^~~~~~~~~
M:/Development/mingw64/bin/../lib/clang/3.0/../../../x86_64-w64-mingw32/include\wchar.h:732:18: note: candidate function not viable: no known conversion from 'char **' to 'const char **restrict' for 2nd argument;
  size_t __cdecl mbsrtowcs(wchar_t * __restrict__ _Dest,const char ** __restrict__ _PSrc,size_t _Count,mbstate_t * __restrict__ _State) __MINGW_ATTRIB_DEPRECATED_SEC_WARN;
                 ^

EDIT2: To fix what was wrong Re. Matthieu, I have changed my naive implementation like so:

size_t mbsnrtowcs( wchar_t *__restrict__ dst, const char **__restrict__ src,
                   size_t nmc, size_t len, mbstate_t *__restrict__ ps )
{
    char* local_src = new char[nmc+1];
    char* nmcsrc = local_src;
    strncpy( nmcsrc, *src, nmc );
    nmcsrc[nmc] = '\0';
    const size_t result = mbsrtowcs( dst, const_cast<const char **>(&nmcsrc), len, ps );
    // propagate error
    if( nmcsrc == NULL )
        *src = NULL;
    delete[] local_src;
    return result;
}

And I have added a FIXME saying that the proper way to do this would be implementing it via mbrtowc.

rubenvb
  • 74,642
  • 33
  • 187
  • 332
  • According to `man mbsrtowcs`, the signature of that function is `(wchar *, const char **, size_t, mbstate_t *)`. There's no `const wchar_t **` in there. – Kerrek SB Sep 23 '11 at 11:41
  • Kerrek: that's what I meant. I also need the `wchar_t` version, hence the typo. Fixed that in the question. – rubenvb Sep 23 '11 at 11:44
  • But then there's no problem, is there? You just want something *more* constant. – Kerrek SB Sep 23 '11 at 11:45
  • Kerrek: Clang thinks there is. Note that these functions are declared with `restrict` keywords... – rubenvb Sep 23 '11 at 11:49
  • There's no `restrict` in C++11 (nor in any C++). You're dealing with compiler extensions at best. E.g. [see here](http://stackoverflow.com/questions/6434549/does-c11-add-the-c99-restrict-specifier-if-not-why-not/6434582#6434582). – Kerrek SB Sep 23 '11 at 11:52

2 Answers2

4

There is an issue here (not really related to the const I think).

mbsrtowcs might set *src to NULL.

mbsnrtowcs is supposed to do the same, of course.

However for you here it causes a 2 bugs:

  • Because you created a local alias, if mbsrtowcs set *src to NULL it is not reflected for the caller of mbsnrtowcs
  • mbsnrtowcs has a memory leak if nmcsrc is set to NULL

Perhaps that simply biting the bullet and implementing mbsnrtowcs in terms of mbrtowc would be more efficient ?

If you need to implement both (as I guess you do), you could also reverse the problem on its head and implement mbsrtowcs in terms of mbsnrtowcs simply by calling strlen at the beginning without loss of generality (and avoid the copy).

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • Good point; you actually need to implement the function logic correctly. Making a copy of `nmsrc` would be a start. – Kerrek SB Sep 23 '11 at 12:37
  • @Kerrek: Yes, make it correct, then make fast. Saving `nmsrc` to be sure to `delete` the allocated buffer and being able to check if it's been set to `NULL` would work. On a related note I still haven't found why C++ does not allow the `const` deduction here. I think Johannes provided a demo of how this could be abused, but I am not sure :/ – Matthieu M. Sep 23 '11 at 12:43
  • I understand mine is a naive and bad implementation, but I really don't trust myself in these things to do otherwise. libc++ is opensource, so if you feel compelled ;)? I have updated my question with NULL handling. I think everything should be fine now? I hope I didn't mess up the pointer alias... – rubenvb Sep 23 '11 at 12:44
  • @MatthieuM: No idea if this would be a desirable extension, but at present you're trying to deduce `U*` from `T*`, which just isn't allowed - you can only deduce `const T` from `T`. – Kerrek SB Sep 23 '11 at 12:48
  • @rubenvb: I know, I've been watching your discussions with Howard on the list, awesome job by the way ;) I already have too many projects on my plate though :/ – Matthieu M. Sep 23 '11 at 12:57
1

There's no problem, since the function expects a more restrictive interface. So you can just cast the pointer:

const size_t result = std::mbsrtowcs(dst, const_cast<const char **>(&nmcsrc), len, ps);

I'm not sure about restrict; you may need to add that to the cast as well (not on GCC, though) - that's a compiler-dependent matter, though, as there is no restrict in C++.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084