[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 07 of 10] xl: Fix 'script' param parsing in network-attach
On Sat, 2011-06-04 at 00:49 +0100, Marek Marczykowski wrote: > On 03.06.2011 10:16, Ian Campbell wrote: > > On Thu, 2011-06-02 at 23:35 +0100, Marek Marczykowski wrote: > >> # HG changeset patch > >> # User Marek Marczykowski <marmarek@xxxxxxxxxxxx> > >> # Date 1306963105 -7200 > >> # Node ID 6811aa543e69379557ff7391ea3db8a5e7f7dde0 > >> # Parent eb7216a75b7d7a5de93c717401b447545022b582 > >> xl: Fix 'script' param parsing in network-attach > >> > >> Fix 'script=' string length > >> > >> Signed-off-by: Marek Marczykowski <marmarek@xxxxxxxxxxxx> > >> > >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > >> --- a/tools/libxl/xl_cmdimpl.c > >> +++ b/tools/libxl/xl_cmdimpl.c > >> @@ -4285,9 +4285,9 @@ > >> } else if (!strncmp("ip=", *argv, 3)) { > >> free(nic.ip); > >> nic.ip = strdup((*argv) + 3); > >> - } else if (!strncmp("script=", *argv, 6)) { > >> + } else if (!strncmp("script=", *argv, 7)) { > >> free(nic.script); > >> - nic.script = strdup((*argv) + 6); > >> + nic.script = strdup((*argv) + 7); > > > > Good catch. > > > > The pre-existing use of all those > > strncmp(A, *argv, open-coded-sizeof(A)) > > must be a source of many such errors. A helper function (or macro) is > > probably the way to go. Do you fancy coding that up? > > And the same size must be used in strdup offset... > Maybe something like this (solving also problem in 06 patch): > ----------- > #define COMPARE_AND_REPLACE(pattern, dest) \ > } else if (!strncmp(pattern, *argv, sizeof(pattern))) { \ > free(dest); \ > dest = strdup((*argv) + sizeof(pattern)); > > (...) > COMPARE_AND_REPLACE("script=", nic.script) > COMPARE_AND_REPLACE("ip=", nic.ip) > --------- > Looks weird because of no semicolon at the end of lines, but should > works. Generally you can fix the semi-colon thing by omitting the last one from the macro itself. > Unfortunately cannot be used in all places (like one below)... > What do you think? How about (untested), requires a local "char *oparg": static int match_option_size(const char *prefix, size_t len, char *arg, char **argopt) { int rc = strncmp(prefix, argv, len); if (!rc) *argopt = argv+len; return !rc } #define match_option(_prefix, _arg, _oparg) \ match_option_size((_prefix), (_arg), sizeof((_prefix)), &(_oparg)); static void replace_string(char **str, const char *val) { free(*str);\ *str = strdup(val); } then you have: else if (match_option("script=", *argv, oparg)) replace_string(&nic.script, oparg); else if (match_option("backend=", *argv, oparg)) { if(libxl_name_to_domid(&ctx, oparg, &val)) { fprintf(stderr, "Specified backend domain does not exist, defaulting to Dom0\n"); ... } } with a bit of macro pasting in match_option you can probably push the "=" down into the macro so each callsite doesn't need it. Ian. > > > > > Ian. > > > >> } else if (!strncmp("backend=", *argv, 8)) { > >> if(libxl_name_to_domid(&ctx, ((*argv) + 8), &val)) { > >> fprintf(stderr, "Specified backend domain does not exist, > >> defaulting to Dom0\n"); > > > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |