[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