[PATCH v2] msvcrt: Introduce vsprintf & vswprintf helper functions.

Piotr Caban piotr.caban at gmail.com
Fri Jan 19 11:47:24 CST 2018


Hi Gijs,

It might be better to add some tests and fix error conditions first. It 
will be easier to see how to implement the helper/if one helper is 
suitable in this case). Some quick testing shows that there are 3 
different behaviors when buffer is too small (sprintf, sprintf_p and 
sprintf_s).

On 01/14/18 03:45, Gijs Vermeulen wrote:
> Signed-off-by: Gijs Vermeulen <gijsvrm at gmail.com>
> ---
>   dlls/msvcrt/wcs.c | 318 +++++++++++++++++++++++-------------------------------
>   1 file changed, 132 insertions(+), 186 deletions(-)
> 
> diff --git a/dlls/msvcrt/wcs.c b/dlls/msvcrt/wcs.c
> index 098e0d5ba8..fb1173c859 100644
> --- a/dlls/msvcrt/wcs.c
> +++ b/dlls/msvcrt/wcs.c
> @@ -720,20 +720,117 @@ printf_arg arg_clbk_positional(void *ctx, int pos, int type, __ms_va_list *valis
>       return args[pos];
>   }
>   
> +static int vsprintf_helper(DWORD options, char *str, MSVCRT_size_t sizeOfBuffer, MSVCRT_size_t count,
> +        const char *format, MSVCRT__locale_t locale, __ms_va_list valist)
> +{
> +    static const char nullbyte = '\0';
> +    printf_arg args_ctx[MSVCRT__ARGMAX+1];
> +    struct _str_ctx_a puts_ctx;
> +    int len, ret;
> +    BOOL postional, secure, initially_positional;
> +
> +    len = sizeOfBuffer;
> +    postional = options & MSVCRT_PRINTF_POSITIONAL_PARAMS;
> +    initially_positional = postional;
> +    secure = options & MSVCRT_PRINTF_INVOKE_INVALID_PARAM_HANDLER;
> +
> +    if(secure && !postional) > +        if(sizeOfBuffer>count+1 && count!=-1) len = count+1;
Please use MSVCRT__TRUNCATE instead of -1 here. The if condition can be 
also simplified if _sprintf_p behavior is fixed when too small buffer is 
passed.

You can see the incorrect behavior with following test:
errno = 0xdeadbeef;
memset(buf, 'a', sizeof(buf));
ret = _vsprintf_p_wrapper(buf, 2, "%d", 1234);
ok(ret == -1, "ret =  %d\n", ret);
ok(errno == ERANGE, "errno = %d\n", errno);
ok(!memcmp(buf, "1", 2), "buf = %s\n", buf);

> +    puts_ctx.len = len;
> +    puts_ctx.buf = str;
> +
> +    if(postional) {
> +        memset(args_ctx, 0, sizeof(args_ctx));
> +
> +        ret = create_positional_ctx_a(args_ctx, format, valist);
> +        if(ret < 0) {
> +            MSVCRT__invalid_parameter(NULL, NULL, NULL, 0, 0);
> +            *MSVCRT__errno() = MSVCRT_EINVAL;
> +            return ret;
> +        } else if(!ret)
> +            postional = FALSE;
> +    }
> +
> +    ret = pf_printf_a(puts_clbk_str_a,
> +            &puts_ctx, format, locale, options,
> +            postional ? arg_clbk_positional : arg_clbk_valist,
> +            postional ? args_ctx : NULL,
> +            postional ? NULL : &valist);
> +    puts_clbk_str_a(&puts_ctx, 1, &nullbyte);
> +
> +    if((secure && !initially_positional) && (ret<0 || ret==len)) {
> +        if(count!=MSVCRT__TRUNCATE && count>sizeOfBuffer) {
You should probably check if count>=sizeOfBuffer.

Thanks,
Piotr



More information about the wine-devel mailing list