<div dir="ltr">Hi Simon,<br><br>On Fri, Jan 31, 2020 at 6:03 PM Simon Marchi <<a href="mailto:simark@simark.ca">simark@simark.ca</a>> wrote:<br>><br>> Hi Maxime,<br>><br>> Tip for next time: this mailing is used for many projects, so it helps<br>> to include the project name in the patch prefix.  You can do so by using<br>><br>>   --subject-prefix="PATCH lttng-ust"<br>><br>> on you git-send-email command line.<div><br></div><div>Will do!</div><div><br>><br>> On 2020-01-31 4:55 p.m., Maxime Roussin-Belanger wrote:<br>> > vtracef accepts a va_list argument to simplify tracing<br>> > functions which use a va_list<br>> ><br>> > Here's an example from wpa_supplicant that I wanted to<br>> > trace:<br>> ><br>> > void wpa_debug(int level, const char* fmt, ...) {<br>> ><br>> >     va_list ap;<br>> >     va_start(ap, fmt);<br>> ><br>> >     ...<br>> >     // The call I want to easily trace with vtracef<br>> >     vprintf(fmt, ap);<br>> ><br>> >     ...<br>> >     va_end(ap);<br>> > }<br>> ><br>> > wpa_debug is used a fair amount and it would be annoying to<br>> > replace all the wpa_debug calls with tracef.<br>> ><br>> > With vtracef, it simplifies the find and replace effort by<br>> > only changing it at one place.<br>><br>> The shortcoming, as you mentiond on IRC, is that it doesn't support SDT integration.  The tracef<br>> macro call has a `STAP_PROBEV` call, but it doesn't seem possible to use that here.  The number<br>> of arguments in a va_list is known at runtime (in fact it's not really known by the va_list itself),<br>> whereas STAP_PROBEV wants to know the number of arguments at compile time.<br>><br>> I think it would still be useful to have vtracef, for the use case explained above, but have it<br>> documented that it is not integrated with the stap probes (unless somebody has a magic idea that<br>> would make it work).  But that will be for the maintainer to decide.</div><div><br></div><div>Where would that be? In the man page? If so it would be the first time editing/creating a man page.</div><div>It seems fairly simple, but I probably need some guidance on where to start.</div><div><br>><br>> Note that if the feature is accepted, it will require at least a man page update. An update in<br>> doc/examples/demo-tracef/demo-tracef.c would also be nice, to have an example of how it can be used.</div><div><br></div><div>Looking at the example, maybe I should create another file demo-vtracef.c?</div><div><br></div><div>Max.</div><div><br>><br>> Simon<br>><br>> ><br>> > Signed-off-by: Maxime Roussin-Belanger <<a href="mailto:maxime.roussinbelanger@gmail.com">maxime.roussinbelanger@gmail.com</a>><br>> > ---<br>> >  include/lttng/tracef.h | 10 ++++++++++<br>> >  liblttng-ust/tracef.c  | 18 +++++++++++-------<br>> >  2 files changed, 21 insertions(+), 7 deletions(-)<br>> ><br>> > diff --git a/include/lttng/tracef.h b/include/lttng/tracef.h<br>> > index 0c59c9ae..e3a7587d 100644<br>> > --- a/include/lttng/tracef.h<br>> > +++ b/include/lttng/tracef.h<br>> > @@ -32,13 +32,23 @@ extern "C" {<br>> >  extern<br>> >  void _lttng_ust_tracef(const char *fmt, ...);<br>> > <br>> > +extern<br>> > +void _lttng_ust_vtracef(const char *fmt, va_list ap);<br>> > +<br>> >  #define tracef(fmt, ...)                                             \<br>> >       do {                                                            \<br>> >               LTTNG_STAP_PROBEV(tracepoint_lttng_ust_tracef, event, ## __VA_ARGS__); \<br>> >               if (caa_unlikely(__tracepoint_lttng_ust_tracef___event.state)) \<br>> >                       _lttng_ust_tracef(fmt, ## __VA_ARGS__);         \<br>> >       } while (0)<br>> > <br>> > +#ifndef LTTNG_UST_HAVE_SDT_INTEGRATION<br>> > +#define vtracef(fmt, ap)                                             \<br>> > +     do {                                                            \<br>> > +             if (caa_unlikely(__tracepoint_lttng_ust_tracef___event.state)) \<br>> > +                     _lttng_ust_vtracef(fmt, ap);            \<br>> > +     } while (0)<br>> > +#endif<br>><br>> If we end up with what I suggested above (vtracef documented not to integrate an<br>> stap probe), this #ifndef would go away,<br>><br>> >  #ifdef __cplusplus<br>> >  }<br>> >  #endif<br>> > diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c<br>> > index ea98e43e..2a809eed 100644<br>> > --- a/liblttng-ust/tracef.c<br>> > +++ b/liblttng-ust/tracef.c<br>> > @@ -29,20 +29,24 @@<br>> >  #define TRACEPOINT_DEFINE<br>> >  #include "lttng-ust-tracef-provider.h"<br>> > <br>> > -void _lttng_ust_tracef(const char *fmt, ...)<br>> > +void _lttng_ust_vtracef(const char *fmt, va_list ap)<br>> >  {<br>> > -     va_list ap;<br>> >       char *msg;<br>> > -     int len;<br>> > -<br>> > -     va_start(ap, fmt);<br>> > -     len = vasprintf(&msg, fmt, ap);<br>> > +     const int len = vasprintf(&msg, fmt, ap);<br>> >       /* len does not include the final \0 */<br>> >       if (len < 0)<br>> > -             goto end;<br>> > +             return;<br>><br>> We try to have a single exit point in each function, to facilitate<br>> resource management in the case of errors, so please keep the goto<br>> end pattern:<br>><br>>   ...<br>><br>>   if (len < 0)<br>>     goto end;<br>><br>>   ...<br>><br>> end:<br>>   return<br>> }<br>><br>> >       __tracepoint_cb_lttng_ust_tracef___event(msg, len,<br>> >               LTTNG_UST_CALLER_IP());<br>> >       free(msg);<br>> > +}<br>> > +<br>> > +void _lttng_ust_tracef(const char *fmt, ...)<br>> > +{<br>> > +     va_list ap;<br>> > +<br>> > +     va_start(ap, fmt);<br>> > +     _lttng_ust_vtracef(fmt, ap);<br>> >  end:<br>><br>> You can remove the end label here.<br>><br>> Simon</div></div>