Bug 99693

Created attachment 129362 [details] [review]
tools: Improve argument validation in dbus-spam

Check that at most one argument which sets the payload is provided, so
the allocated payload is not overwritten and leaked.

Coverity ID: 54759

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Comment on attachment 129362 [details] [review]
tools: Improve argument validation in dbus-spam

Review of attachment 129362 [details] [review]:
-----------------------------------------------------------------

::: tools/dbus-spam.c
@@ +186,5 @@
> +           strcmp (arg, "--random-size") == 0))
> +        {
> +          fprintf (stderr, "At most one of --payload, --stdin, --message-stdin "
> +                           "and --random-size may be specified\n");
> +          exit (1);

I'd slightly prefer exit status 2, which I believe is semi-common for distinguishing invalid arguments from a functional failure (for instance it's what Python argparse uses) and is consistently used in this tool.

I'd slightly prefer printing usage: usage (2); which again is consistently used in this tool.

(In reply to Simon McVittie from comment #2)
> Comment on attachment 129362 [details] [review] [review]
> tools: Improve argument validation in dbus-spam
> 
> Review of attachment 129362 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: tools/dbus-spam.c
> @@ +186,5 @@
> > +           strcmp (arg, "--random-size") == 0))
> > +        {
> > +          fprintf (stderr, "At most one of --payload, --stdin, --message-stdin "
> > +                           "and --random-size may be specified\n");
> > +          exit (1);
> 
> I'd slightly prefer exit status 2, which I believe is semi-common for
> distinguishing invalid arguments from a functional failure (for instance
> it's what Python argparse uses) and is consistently used in this tool.

Ah, yes.

> I'd slightly prefer printing usage: usage (2); which again is consistently
> used in this tool.

And dropping the specific error message, or printing usage(2) in addition to it?

(In reply to Philip Withnall from comment #3)
> > I'd slightly prefer printing usage: usage (2); which again is consistently
> > used in this tool.
> 
> And dropping the specific error message, or printing usage(2) in addition to
> it?

I meant in addition. (But if you think the result looks stupid, just print the specific message and exit 2.)

Created attachment 129389 [details] [review]
tools: Improve argument validation in dbus-spam

Check that at most one argument which sets the payload is provided, so
the allocated payload is not overwritten and leaked.

Coverity ID: 54759

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Looks good, thanks. Will be in my next batch of applying patches.

Fixed in git for 1.11.10, thanks