Marek Marczykowski
2011-Jun-12 18:54 UTC
[Xen-devel] [PATCH] libxl: off by one fix for new network-attach args parsing
# HG changeset patch
# User Marek Marczykowski <marmarek@mimuw.edu.pl>
# Date 1307904815 -7200
# Node ID 764214d25cd9825925aeb846a6bdec56c9a5259a
# Parent 262a414e7e37541ff3c1346b316487606e9e736f
libxl: off by one fix for new network-attach args parsing
sizeof(const string) returns string length WITH \0 terminator.
Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>
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
@@ -1238,7 +1238,7 @@ static int match_option_size(const char
return !rc;
}
#define match_option(_prefix, _arg, _oparg) \
- match_option_size((_prefix "="), sizeof((_prefix)) + 1, (_arg),
&(_oparg))
+ match_option_size((_prefix "="), sizeof((_prefix)), (_arg),
&(_oparg))
static void replace_string(char **str, const char *val)
{
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-17 17:58 UTC
Re: [Xen-devel] [PATCH] libxl: off by one fix for new network-attach args parsing
Marek Marczykowski writes ("[Xen-devel] [PATCH] libxl: off by one fix for
new network-attach args parsing"):> 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
> @@ -1238,7 +1238,7 @@ static int match_option_size(const char
> return !rc;
> }
> #define match_option(_prefix, _arg, _oparg) \
> - match_option_size((_prefix "="), sizeof((_prefix)) + 1,
(_arg), &(_oparg))
> + match_option_size((_prefix "="), sizeof((_prefix)), (_arg),
&(_oparg))
I haven''t applied your patch yet, so could you fold this in and resend
please ?
Why do you use _''d names for the macro formal parameters ? This is
not our usual coding style.
Finally can you please make sure that your patch has lines of only
around 75 characters, and call the macro MATCH_OPTION since it''s not a
normal function.
thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-21 16:49 UTC
[Xen-devel] [PATCH RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
# HG changeset patch
# User Marek Marczykowski <marmarek@mimuw.edu.pl>
# Date 1307145042 -7200
# Node ID f49657a37635b6b5a2ee0b41bf03e2eb15aedcf2
# Parent 9fe949c7ab9601bb5500a53c538f7a23b61e1bcb
xl: Use macros for param parsing in network-attach, to prevent use explicit
sizeof("param")
''script='' length was wrong... Replaced calls to
strncmp("param", *argv,
explicit sizeof("param")) with macro and helper function to extract
parameter
value. Also introduce replace_string function to simplify code.
Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl>
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
@@ -1229,6 +1229,24 @@ static int handle_domain_death(libxl_ctx
return restart;
}
+/* for now used only by main_networkattach, but can be reused elsewhere */
+static int match_option_size(const char *prefix, size_t len,
+ char *arg, char **argopt)
+{
+ int rc = strncmp(prefix, arg, len);
+ if (!rc) *argopt = arg+len;
+ return !rc;
+}
+#define MATCH_OPTION(prefix, arg, oparg) \
+ match_option_size((prefix "="), sizeof((prefix)), (arg),
&(oparg))
+
+static void replace_string(char **str, const char *val)
+{
+ free(*str);
+ *str = strdup(val);
+}
+
+
static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event *event,
libxl_domain_config *d_config, libxl_dominfo *info)
{
@@ -3925,7 +3943,7 @@ int main_networkattach(int argc, char **
{
int opt;
libxl_device_nic nic;
- char *endptr;
+ char *endptr, *oparg;
const char *tok;
int i;
unsigned int val;
@@ -3944,17 +3962,17 @@ int main_networkattach(int argc, char **
}
libxl_device_nic_init(&nic, -1);
for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
- if (!strncmp("type=", *argv, 5)) {
- if (!strncmp("vif", (*argv) + 5, 4)) {
+ if (MATCH_OPTION("type", *argv, oparg)) {
+ if (!strcmp("vif", oparg)) {
nic.nictype = LIBXL_NIC_TYPE_VIF;
- } else if (!strncmp("ioemu", (*argv) + 5, 5)) {
+ } else if (!strcmp("ioemu", oparg)) {
nic.nictype = LIBXL_NIC_TYPE_IOEMU;
} else {
fprintf(stderr, "Invalid parameter
`type''.\n");
return 1;
}
- } else if (!strncmp("mac=", *argv, 4)) {
- tok = strtok((*argv) + 4, ":");
+ } else if (MATCH_OPTION("mac", *argv, oparg)) {
+ tok = strtok(oparg, ":");
for (i = 0; tok && i < 6; tok = strtok(NULL,
":"), ++i) {
val = strtoul(tok, &endptr, 16);
if ((tok == endptr) || (val > 255)) {
@@ -3963,29 +3981,24 @@ int main_networkattach(int argc, char **
}
nic.mac[i] = val;
}
- } else if (!strncmp("bridge=", *argv, 7)) {
- free(nic.bridge);
- nic.bridge = strdup((*argv) + 7);
- } else if (!strncmp("ip=", *argv, 3)) {
- free(nic.ip);
- nic.ip = strdup((*argv) + 3);
- } else if (!strncmp("script=", *argv, 6)) {
- free(nic.script);
- nic.script = strdup((*argv) + 6);
- } else if (!strncmp("backend=", *argv, 8)) {
- if(libxl_name_to_domid(ctx, ((*argv) + 8), &val)) {
+ } else if (MATCH_OPTION("bridge", *argv, oparg)) {
+ replace_string(&nic.bridge, oparg);
+ } else if (MATCH_OPTION("ip", *argv, oparg)) {
+ replace_string(&nic.ip, oparg);
+ } 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");
val = 0;
}
nic.backend_domid = val;
- } else if (!strncmp("vifname=", *argv, 8)) {
- free(nic.ifname);
- nic.ifname = strdup((*argv) + 8);
- } else if (!strncmp("model=", *argv, 6)) {
- free(nic.model);
- nic.model = strdup((*argv) + 6);
- } else if (!strncmp("rate=", *argv, 5)) {
- } else if (!strncmp("accel=", *argv, 6)) {
+ } else if (MATCH_OPTION("vifname", *argv, oparg)) {
+ replace_string(&nic.ifname, oparg);
+ } else if (MATCH_OPTION("model", *argv, oparg)) {
+ replace_string(&nic.model, oparg);
+ } else if (MATCH_OPTION("rate", *argv, oparg)) {
+ } else if (MATCH_OPTION("accel", *argv, oparg)) {
} else {
fprintf(stderr, "unrecognized argument `%s''\n",
*argv);
return 1;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 14:26 UTC
Re: [Xen-devel] [PATCH RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
Marek Marczykowski writes ("[Xen-devel] [PATCH RESENT] xl: Use macros for
param parsing in network-attach, to prevent use explicit
sizeof("param")"):> xl: Use macros for param parsing in network-attach, to prevent use explicit
sizeof("param")
>
> ''script='' length was wrong... Replaced calls to
strncmp("param", *argv,
> explicit sizeof("param")) with macro and helper function to
extract parameter
> value. Also introduce replace_string function to simplify code.
I think I''m expecting an updated version of this ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-27 14:37 UTC
Re: [Xen-devel] [PATCH RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
On 27.06.2011 16:26, Ian Jackson wrote:> Marek Marczykowski writes ("[Xen-devel] [PATCH RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")"): >> xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param") >> >> ''script='' length was wrong... Replaced calls to strncmp("param", *argv, >> explicit sizeof("param")) with macro and helper function to extract parameter >> value. Also introduce replace_string function to simplify code. > > I think I''m expecting an updated version of this ?This is an updated version... -- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-27 16:21 UTC
Re: [Xen-devel] [PATCH RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
Marek Marczykowski writes ("Re: [Xen-devel] [PATCH RESENT] xl: Use macros
for param parsing in network-attach, to prevent use explicit
sizeof("param")"):> This is an updated version...
Oh, yes, so it was. I had an off-by-one error looking for the
off-by-one error you fixed :-).
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel