Re: [v2 PATCH 1/4] shcore: Implement a bunch of string conversion functions.

[Nikolay Sivov]

On 11/29/18 9:34 AM, Dmitry Timoshkov wrote:
> Nikolay Sivov  wrote:
> > > > +DWORD WINAPI SHAnsiToAnsi(const char *src, char *dest, int dest_len)
> > > > +{
> > > > - DWORD ret = 0;
> > > > -
> > > > - TRACE("(%s, %p, %d)\n", debugstr_a(src), dest, dest_len);
> > > > -
> > > > - if (!src || !dest || dest_len <= 0)
> > > > -
> > > >    return 0;
> > > > -
> > > > - while (dest_len > 1 && *src)
> > > > - {
> > > > -
> > > >    dest_len--;
> > > > -
> > > >    *dest++ = *src++;
> > > > -
> > > >    ret++;
> > > > - }
> > > > -
> > > > - if (dest_len)
> > > > - {
> > > > -
> > > >    *dest = 0;
> > > > -
> > > >    ret++;
> > > > - }
> > > > -
> > > > - return ret;
> > > >
> > > > +}
> > >
> > > Still the same duplication without a reasonable explanation.
> >
> > It's used to get correct return value.
>
> lstrcpynA(dst, src, dstlen);
> return strlen(dst) + 1;

This won't work because it should return 0 in case of truncation. Even 
if it did work, this duplicates a loop.

I'll add more tests.
> > > > +DWORD WINAPI SHUnicodeToAnsi(const WCHAR *src, char *dest, int dest_len)
> > > > +{
> > > > - int required = 1;
> > > > -
> > > > - TRACE("(%s, %p, %d)\n", debugstr_w(src), dest, dest_len);
> > > > -
> > > > - if (!dest || !dest_len)
> > > > -
> > > >    return 0;
> > > > -
> > > > - if (src)
> > > > - {
> > > > -
> > > >    required = WideCharToMultiByte(CP_ACP, 0, src, -1, NULL, 0, NULL, NULL);
> > > > -
> > > >    WideCharToMultiByte(CP_ACP, 0, src, -1, dest, dest_len, NULL, NULL);
> > >
> > > It should be possible to avoid doing the conversion twice.
> >
> > Second call returns 0 for shorter buffer.
>
> ret = WideCharToMultiByte(CP_ACP, 0, src, -1, dest, dest_len, NULL, NULL);
> if (!ret) dest[dest_len - 1] = 0;
> return dest_len;

Ok.
> > If you have actual suggestion I'm sure nobody would mind if you posted it.
>
> Sure.