| Message ID | 1344577132-19796-2-git-send-email-michael@ellerman.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 10, 2012 at 1:38 PM, Michael Ellerman <michael@ellerman.id.au> wrote: > Currently the parsing of network command line parameters doesn't check > for unknown parameters. > > For example "-n mod=none" will cause kvmtool to setup TAP networking. > > Save users from their typos by checking for unknown parameters and bailing. > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au> > --- > tools/kvm/builtin-run.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c > index a36bd00..9e5c1d4 100644 > --- a/tools/kvm/builtin-run.c > +++ b/tools/kvm/builtin-run.c > @@ -257,7 +257,8 @@ static int set_net_param(struct virtio_net_params *p, const char *param, > p->vhost = atoi(val); > } else if (strcmp(param, "fd") == 0) { > p->fd = atoi(val); > - } > + } else > + die("Unknown network parameter %s", param); we need braces here: + } else { + die("Unknown network parameter %s", param); + } Otherwise, ACK.
On Sat, 2012-08-11 at 08:21 +0800, Asias He wrote: > On Fri, Aug 10, 2012 at 1:38 PM, Michael Ellerman > <michael@ellerman.id.au> wrote: > > Currently the parsing of network command line parameters doesn't check > > for unknown parameters. > > > > For example "-n mod=none" will cause kvmtool to setup TAP networking. > > > > Save users from their typos by checking for unknown parameters and bailing. > > > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au> > > --- > > tools/kvm/builtin-run.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c > > index a36bd00..9e5c1d4 100644 > > --- a/tools/kvm/builtin-run.c > > +++ b/tools/kvm/builtin-run.c > > @@ -257,7 +257,8 @@ static int set_net_param(struct virtio_net_params *p, const char *param, > > p->vhost = atoi(val); > > } else if (strcmp(param, "fd") == 0) { > > p->fd = atoi(val); > > - } > > + } else > > + die("Unknown network parameter %s", param); > > we need braces here: We don't _need_ braces, but I assume you mean you'd prefer braces? cheers -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 13, 2012 at 6:31 AM, Michael Ellerman <michael@ellerman.id.au> wrote: >> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c >> > index a36bd00..9e5c1d4 100644 >> > --- a/tools/kvm/builtin-run.c >> > +++ b/tools/kvm/builtin-run.c >> > @@ -257,7 +257,8 @@ static int set_net_param(struct virtio_net_params *p, const char *param, >> > p->vhost = atoi(val); >> > } else if (strcmp(param, "fd") == 0) { >> > p->fd = atoi(val); >> > - } >> > + } else >> > + die("Unknown network parameter %s", param); >> >> we need braces here: > > We don't _need_ braces, but I assume you mean you'd prefer braces? This is Linux coding style so we don't prefer them either. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 13, 2012 at 3:27 PM, Pekka Enberg <penberg@kernel.org> wrote: > On Mon, Aug 13, 2012 at 6:31 AM, Michael Ellerman > <michael@ellerman.id.au> wrote: >>> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c >>> > index a36bd00..9e5c1d4 100644 >>> > --- a/tools/kvm/builtin-run.c >>> > +++ b/tools/kvm/builtin-run.c >>> > @@ -257,7 +257,8 @@ static int set_net_param(struct virtio_net_params *p, const char *param, >>> > p->vhost = atoi(val); >>> > } else if (strcmp(param, "fd") == 0) { >>> > p->fd = atoi(val); >>> > - } >>> > + } else >>> > + die("Unknown network parameter %s", param); >>> >>> we need braces here: >> >> We don't _need_ braces, but I assume you mean you'd prefer braces? > > This is Linux coding style so we don't prefer them either. Documentation/CodingStyle says: ''' Do not unnecessarily use braces where a single statement will do. if (condition) action(); and if (condition) do_this(); else do_that(); This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches: if (condition) { do_this(); do_that(); } else { otherwise(); } ''' we have a multiple line branch in set_net_param(), so I think it's better to have the braces for the last die() branch.
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c index a36bd00..9e5c1d4 100644 --- a/tools/kvm/builtin-run.c +++ b/tools/kvm/builtin-run.c @@ -257,7 +257,8 @@ static int set_net_param(struct virtio_net_params *p, const char *param, p->vhost = atoi(val); } else if (strcmp(param, "fd") == 0) { p->fd = atoi(val); - } + } else + die("Unknown network parameter %s", param); return 0; }
Currently the parsing of network command line parameters doesn't check for unknown parameters. For example "-n mod=none" will cause kvmtool to setup TAP networking. Save users from their typos by checking for unknown parameters and bailing. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- tools/kvm/builtin-run.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)