On 19 Aug 2015 18:56, Dmitry V. Levin wrote: > On Tue, Aug 18, 2015 at 05:03:20PM -0400, Mike Frysinger wrote: > [...] > > * util.c (printxval): Rename to ... > > (printxvals): ... this. Rewrite to be varargs based. > > * xlat/getsockipoptions.in: New xlat list. > > * xlat/getsockipv6options.in, xlat/setsockipoptions.in, > > xlat/setsockipv6options.in: Likewise. > > --- > > RFC: i'm not terribly happy with the printxvals logic. open to suggestions. > > What's it in the printxvals logic that doesn't make you happy?
using varargs all the time when the previous code was just a simple func.
seems like it's adding a good amount of overhead. maybe i'm pessimistic
as i haven't done any perf checks.
> On Wed, Aug 19, 2015 at 09:14:59AM -0400, Mike Frysinger wrote:
> [...]
> > @@ -207,14 +208,26 @@ next_set_bit(const void *bit_array, unsigned cur_bit,
> > unsigned size_bits)
> > * Print entry in struct xlat table, if there.
> > */
> > void
> > -printxval(const struct xlat *xlat, const unsigned int val, const char
> > *dflt)
> > +printxvals(const unsigned int val, const char *dflt, const struct xlat
> > *xlat, ...)
> > {
> > - const char *str = xlookup(xlat, val);
> > + va_list args;
> > + const char *str;
> > +
> > + va_start(args, xlat);
> > + while (xlat) {
> > + str = xlookup(xlat, val);
> > + if (str) {
> > + tprints(str);
> > + return;
> > + }
> >
> > - if (str)
> > - tprints(str);
> > - else
> > - tprintf("%#x /* %s */", val, dflt);
> > + xlat = va_arg(args, const struct xlat *);
> > + if (!xlat) {
> > + tprintf("%#x /* %s */", val, dflt);
> > + break;
> > + }
> > + }
> > + va_end(args);
> > }
> >
> > /*
>
> Wouldn't it be a bit simpler to write it this way:
>
> @@ -207,14 +208,22 @@ next_set_bit(const void *bit_array, unsigned cur_bit,
> unsigned size_bits)
> * Print entry in struct xlat table, if there.
> */
> void
> -printxval(const struct xlat *xlat, const unsigned int val, const char *dflt)
> +printxvals(const unsigned int val, const char *dflt, const struct xlat
> *xlat, ...)
> {
> - const char *str = xlookup(xlat, val);
> + va_list args;
>
> - if (str)
> - tprints(str);
> - else
> - tprintf("%#x /* %s */", val, dflt);
> + va_start(args, xlat);
> + for (; xlat; xlat = va_arg(args, const struct xlat *)) {
> + const char *str = xlookup(xlat, val);
> +
> + if (str) {
> + tprints(str);
> + return;
> + }
> + }
> + va_end(args);
> +
> + tprintf("%#x /* %s */", val, dflt);
> }
seems like it'd work too. although both versions have a bug -- they don't
call va_end before returning. in my version i could turn it into a break,
but this one needs a 2nd call. most likely not a big issue.
-mike
signature.asc
Description: Digital signature
------------------------------------------------------------------------------
_______________________________________________ Strace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/strace-devel
