On 5 September 2018 at 13:03, asimiklit.w...@gmail.com
<asimiklit.w...@gmail.com> wrote:
> Hello,
>
> Thanks for your reply.
>
> I think that we just misunderstand each other)
>
> static inline int
> util_vsnprintf(char *str, size_t size, const char *format, va_list ap)
> {
> /* We need to use _vscprintf to calculate the length as vsnprintf returns
> -1
> * if the number of characters to write is greater than count.
> */
> va_list ap_copy;
> int ret;
> va_copy(ap_copy, ap);
> ret = _vsnprintf(str, size, format, ap);
task 2: use the copied va
> if (ret < 0) {
> ret = _vscprintf(format, ap_copy);
> }
> + va_end(ap_copy);
> return ret;
> }
>
> As far as I understand this fix will just release the resource which was
> allocated locally.
>
> If you care about an 'va_list' invalidation after call of 'util_vsnprintf'
> function
> it works as was. The same result will be on Linux with 'vsnprintf'.
> Please pay attention that the 'va_end' function is called
> only for a copy of input argument not for original.
>
> I guess it can not cause problems.
>
> Please correct me If I am incorrect.
>
Reasonable assumption, which I would [personally] not dare to make
since we're talking about MSVC and C99 compliance.
Looking at MSDN
Beginning with the UCRT in Visual Studio 2015 and Windows 10,
vsnprintf is no longer identical to _vsnprintf. The vsnprintf function
complies with the C99 standard; _vnsprintf is retained for backward
compatibility with older Visual Studio code.
We have various workarounds for missing va_copy [and in general string
handling] which one gets picked where is fairly magical.
> 1. fix util_vsnprintf users which do not va_copy
>
> I found just one case in the mesa source code where 'va_copy'
> should be added. It is in 'util_vasprintf' function.
> Actually there already is the copy of va but for some reason it is not used.
> https://cgit.freedesktop.org/mesa/mesa/tree/src/util/u_string.h#n121
>
Right, I've noticed that one and didn't look if there are more.
> 2. use the copied va when using util_vsnprintf and in the u_string.h
>
> You mean to use 'va_copy' for the each 'util_vsnprintf' function usage
> in mesa source code? I do not see reasons for that.
>
Grepping through for "va_copy" and checking the new one is actually used shows:
src/glx/apple/apple_glx_log.c -> missing va_end
src/intel/tools/imgui/imgui.cpp -> using the orig va, then the copy
src/util/u_string.h: util_vsnprintf -> using the orig va, then the copy
src/util/u_string.h: util_vasprintf -> using the orig va
> (optional) 4. replace opencoded instances of util_vsnprintf
>
> Do you mean find all places where 'vsnprintf' function is used directly
> and replace it by our more compatible 'util_vsnprintf'?
> Like following example:
> https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/imports.c#n256
>
> #ifdef _WIN32
> #define vsnprintf _vsnprintf
> #else
> ....................
> int
> _mesa_vsnprintf(char *str, size_t size, const char *fmt, va_list args)
> {
> return vsnprintf( str, size, fmt, args);
> }
>
> I think it is good idea because win32 '_vsnprintf' function works a bit
> differently
> at least for case when the input buffer size less than the required size:
> windows '_vsnprintf' will return -1 for this case.
> linux 'vsnprintf' will return the required size.
>
That's one case, yes. Skimming through src/intel there is a bunch of
va_start + vsnprintf + va_end which could be swapped with
util_s[n]printf.
HTH
Emil
> P.S. Please use plan-text + interleaved quoting when working with mailing
> lists.
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
