[PATCH/RFC] decode extend getsockopt/setsockopt options

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20150819/116afb44/attachment.bin>