Re: [PATCH v3] ucrtbase: Implement some printf functions

[Piotr Caban]

Hi,

On 10/26/15 16:53, Martin Storsjo wrote:
> --- a/dlls/msvcrt/printf.h
> +++ b/dlls/msvcrt/printf.h
> +static int FUNC_NAME(puts_clbk_str_c99)(void *ctx, int len, const APICHAR *str)

I think that it would be nicer if this function is added in wcs.c. Other 
callback functions are already added to files that uses them (e.g. 
file.c defines callbacks for functions printing to files).
> /*********************************************************************
> -
> -
> __stdio_common_vsprintf (MSVCRT.@)
> - */
>
> +int CDECL MSVCRT__stdio_common_vsprintf( unsigned __int64 options, char *str, MSVCRT_size_t len, const char *format,
> -
>                                     MSVCRT__locale_t locale, __ms_va_list valist )
>
> +{
> - static const char nullbyte = '\0';
> - struct _str_ctx_a ctx = {len, str};
> - int ret;
> -
> - ret = pf_printf_a(options & 2 ? puts_clbk_str_c99_a : puts_clbk_str_a,
> -
>        &ctx, format, locale, FALSE, FALSE, arg_clbk_valist, NULL, &valist);

Is there any guarantee that options can only equal 1 or 2? If not please 
add a FIXME for other values.

Thanks,
Piotr

[PATCH v3] ucrtbase: Implement some printf functions

[Martin Storsjö]

On Tue, 27 Oct 2015, Piotr Caban wrote:
> Hi,
>
> On 10/26/15 16:53, Martin Storsjo wrote:
> > --- a/dlls/msvcrt/printf.h
> > +++ b/dlls/msvcrt/printf.h
> > +static int FUNC_NAME(puts_clbk_str_c99)(void *ctx, int len, const APICHAR 
> > *str)
>
> I think that it would be nicer if this function is added in wcs.c. Other 
> callback functions are already added to files that uses them (e.g. file.c 
> defines callbacks for functions printing to files).

Ok, that makes sense - will do.
> > /*********************************************************************
> > -
> > -
> > __stdio_common_vsprintf (MSVCRT.@)
> > - */
> >
> > +int CDECL MSVCRT__stdio_common_vsprintf( unsigned __int64 options, char 
> > *str, MSVCRT_size_t len, const char *format,
> > -
> >                                     MSVCRT__locale_t locale, 
> >
> > __ms_va_list valist )
> > +{
> > - static const char nullbyte = '\0';
> > - struct _str_ctx_a ctx = {len, str};
> > - int ret;
> > -
> > - ret = pf_printf_a(options & 2 ? puts_clbk_str_c99_a : puts_clbk_str_a,
> > -
> >        &ctx, format, locale, FALSE, FALSE, arg_clbk_valist, NULL, 
> >
> > &valist);
>
> Is there any guarantee that options can only equal 1 or 2? If not please add 
> a FIXME for other values.

It probably can have other values as well - IIRC there are some flags that 
alter the way that the conversion specifiers work as well, and passing 0 
for options also seems to work for ucrtbase.dll - it kinda seems to behave 
like the legacy mode but not quite. (Not sure if that ever actually gets 
called somewhere like that, though.)

I'll add a FIXME for that - thanks!

// Martin

[PATCH v3] ucrtbase: Implement some printf functions

[Martin Storsjö]

On Tue, 27 Oct 2015, Martin Storsjö wrote:
> On Tue, 27 Oct 2015, Piotr Caban wrote:
> > Hi,
> >
> > On 10/26/15 16:53, Martin Storsjo wrote:
> > > --- a/dlls/msvcrt/printf.h
> > > +++ b/dlls/msvcrt/printf.h
> > > +static int FUNC_NAME(puts_clbk_str_c99)(void *ctx, int len, const APICHAR 
> > > *str)
> >
> > I think that it would be nicer if this function is added in wcs.c. Other 
> > callback functions are already added to files that uses them (e.g. file.c 
> > defines callbacks for functions printing to files).
>
> Ok, that makes sense - will do.

Actually, since this function is templated, it has to stay in some .h 
file in order to include it twice - otherwise I'd have to either duplicate 
the _a/_w implementations, or define it as a macro with the templating 
done via macro parameters. Which one of these seem most appealing to you?

// Martin

[PATCH v3] ucrtbase: Implement some printf functions

[Piotr Caban]

On 27/10/15 20:43, Martin Storsjö wrote:
> Actually, since this function is templated, it has to stay in some .h 
> file in order to include it twice - otherwise I'd have to either 
> duplicate the _a/_w implementations, or define it as a macro with the 
> templating done via macro parameters. Which one of these seem most 
> appealing to you?

I think you can duplicate the _a/_w implementations.

Thanks,
Piotr