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
|
Home