[PATCH 04/10] ucrtbase: Handle the swprintf style termination and return values

[Martin Storsjö]

On Mon, 2 Nov 2015, Piotr Caban wrote:

> Hi,
>
> On 11/02/15 11:08, Martin Storsjo wrote:
>> @@ -724,12 +724,13 @@ int CDECL MSVCRT__stdio_common_vsprintf( unsigned 
>> __int64 options, char *str, MS
>>       struct _str_ctx_a ctx = {len, str};
>>       int ret;
>> 
>> -    if (options != UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION &&
>> -        options != UCRTBASE_PRINTF_STANDARD_SNPRINTF_BEHAVIOUR)
>> +    if (options & ~UCRTBASE_PRINTF_TERMINATION_MASK)
>>           FIXME("options %s not handled\n", wine_dbgstr_longlong(options));
>> -    ret = pf_printf_a(options & 
>> UCRTBASE_PRINTF_STANDARD_SNPRINTF_BEHAVIOUR ? puts_clbk_str_c99_a : 
>> puts_clbk_str_a,
>> +    ret = pf_printf_a(options & 
>> UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION ? puts_clbk_str_a : 
>> puts_clbk_str_c99_a,
>>               &ctx, format, locale, FALSE, FALSE, arg_clbk_valist, NULL, 
>> &valist);
>>       puts_clbk_str_a(&ctx, 1, &nullbyte);
>> +    if ((options & UCRTBASE_PRINTF_TERMINATION_MASK) == 0 && ret >= len)
>> +        ret = -2;
>>       return ret;
>>   }
> This code looks like you're trying to make the things other way around.
> Shouldn't UCRTBASE_PRINTF_STANDARD_SNPRINTF_BEHAVIOUR flag impact the
> callback being used? On the other hand
> UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION flag should probably
> only affect terminating NULL related behavior.

Actually, I don't think of them as two separate flags, but as having 3 
different modes (with the fourth, with both flags being set, undefined). 
But I see what you're getting at.

> Maybe following code makes more sense:
> ret = pf_printf_a(options & UCRTBASE_PRINTF_STANDARD_SNPRINTF_BEHAVIOUR ? 
> puts_clbk_str_c99_a : puts_clbk_str_a, &ctx, format, locale, FALSE, FALSE, 
> arg_clbk_valist, NULL, &valist);
> if(puts_clbk_str_a(&ctx, 1, &nullbyte)==-1 && !(options & 
> UCRTBASE_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION)) {
>    if(len) str[len-1] = 0;
>    return -2;
> }
> return ret;
>
> What do you think about it?

I guess this would work as well, although it took me a little while to 
figure out why (why the return -2 case won't be invoked when the standard 
snprintf behaviour is requested - because the nullbyte write will always 
succeed since the c99 callback left space for it).

After thinking another few minutes about it - sure, this is fine for me. 
Should I resend the full patchset, or only 4->10 (at least some of them 
will need some conflict resolution)? If I resend the full patchset, I 
guess I can fold in your sign-offs in 1-3 (as long as I don't modify those 
patches)?

// Martin