Hi, Thanks for your reply.
These WIN32 compat functions seem iffy. > Namely - often we'll issue a va_copy only to use the original va. > As far as I found there is the following reason to use va_copy: https://linux.die.net/man/3/vsnprintf "These functions do not call the va_end macro. Because they invoke the va_arg macro, the value of ap is *undefined after the call*" http://www.cplusplus.com/reference/cstdio/vsnprintf/ "nternally, the function retrieves arguments from the list identified by arg as if va_arg was used on it, and thus the state of *arg is likely to be altered by the call.*" but unfortunately nothing about it on MSDN so I presume the worst case - it works in the same way. So the main reason to use va_copy it is when we need pass our 'va_list' instance to more then one functions because the 'va_list' instance can be changed by the called function. I think that the 'util_vasprintf' on line 116 also should be fixed because it creates the the copy of 'va_list' but passes the original 'va_list' instance into both function. Looking through the rest of the code base: > There are plenty of va_start + util_vsnprintf + va_free cases that > could be replaced with util_snprintf. > I think that I misunderstand you here. I don't know way to do it. For example: > > static inline void > trace_dump_writef(const char *format, ...) > { > static char buf[1024]; > unsigned len; > va_list ap; > va_start(ap, format); > len = util_vsnprintf(buf, sizeof(buf), format, ap); > va_end(ap); > trace_dump_write(buf, len); > } > if I try to replace 3 lines above by util_snprintf what should I pass to parameter '...' ? Could you please give me an example how to do it? There are also some util_vsnprintf users which use va_copy and some > who do not :-\ > How I explained above it is needed because we can not pass the same instance of 'va_list' to multiple functions because 'va_list' instance can be changed by the called function. I will send new patch as soon as we came agreement about it. Please correct me if I am incorrect. Regards, Andrii. On Mon, Sep 3, 2018 at 6:27 PM Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 3 September 2018 at 15:49, <asimiklit.w...@gmail.com> wrote: > > From: Andrii Simiklit <andrii.simik...@globallogic.com> > > > > MSDN: > > "va_end must be called on each argument list that's initialized > > with va_start or va_copy before the function returns." > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107810 > > Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com> > > > > --- > > src/util/u_string.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/util/u_string.h b/src/util/u_string.h > > index ce45430..883fa64 100644 > > --- a/src/util/u_string.h > > +++ b/src/util/u_string.h > > @@ -81,6 +81,7 @@ util_vsnprintf(char *str, size_t size, const char > *format, va_list ap) > > if (ret < 0) { > > ret = _vscprintf(format, ap_copy); > > } > > + va_end(ap_copy); > > return ret; > > These WIN32 compat functions seem iffy. > Namely - often we'll issue a va_copy only to use the original va. > > Looking through the rest of the code base: > There are plenty of va_start + util_vsnprintf + va_free cases that > could be replaced with util_snprintf. > > There are also some util_vsnprintf users which use va_copy and some > who do not :-\ > > I think those should be looked first, otherwise adding this va_end > could easily break things. > > HTH > Emil >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
