Re: [2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> +    len = MultiByteToWideChar( CP_ACP, 0, format, -1, NULL, 0 );
> +    if ((fmt = MSVCRT_malloc( len * sizeof(MSVCRT_wchar_t) )))
> +    {
> +        MultiByteToWideChar( CP_ACP, 0, format, -1, fmt, len );
> +
> +        if ((len = MSVCRT_wcsftime( s, max, fmt, mstm )))
> +        {
> +            len = WideCharToMultiByte( CP_ACP, 0, s, len, str, max, NULL, NULL );
> +            if (len < max) str[len] = 0;
> +        }

The overflow handling still looks suspicious. It probably needs some
more test cases.

-- 
Alexandre Julliard
julliard(a)winehq.org

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> The overflow handling still looks suspicious. It probably needs some
> more test cases.

I'm probably missing something, why new attempt is marked as pending?

-- 
Dmitry.

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > The overflow handling still looks suspicious. It probably needs some
> > more test cases.
>
> I'm probably missing something, why new attempt is marked as pending?

It's still broken. Consider what happens if there's no room for the
final null.

-- 
Alexandre Julliard
julliard(a)winehq.org

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> It's still broken. Consider what happens if there's no room for the
> final null.

Is there that kind of problem in the new tests?

-- 
Dmitry.

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > It's still broken. Consider what happens if there's no room for the
> > final null.
>
> Is there that kind of problem in the new tests?

No but if the tests didn't catch that problem it's probably a sign that
they are not extensive enough.

-- 
Alexandre Julliard
julliard(a)winehq.org

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Dmitry Timoshkov]

Alexandre Julliard <julliard(a)winehq.org> wrote:
> > Is there that kind of problem in the new tests?
>
> No but if the tests didn't catch that problem it's probably a sign that
> they are not extensive enough.

New tests exercising any desirable behaviour could be added at any point,
it never was a reason to reject a test case before. If there is a need
for some specific test it's usually enough to suggest what you'd like
to see tested.

-- 
Dmitry.

[2/2] msvcrt: Forward strftime() to wcsftime(). Take 2.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
> Alexandre Julliard <julliard(a)winehq.org> wrote:
> > > Is there that kind of problem in the new tests?
> >
> > No but if the tests didn't catch that problem it's probably a sign that
> > they are not extensive enough.
>
> New tests exercising any desirable behaviour could be added at any point,
> it never was a reason to reject a test case before. If there is a need
> for some specific test it's usually enough to suggest what you'd like
> to see tested.

I didn't reject the test case. It's just pending until you get the code
right, which may require revisiting the tests.

-- 
Alexandre Julliard
julliard(a)winehq.org