* Sasha Levin <levinsasha928@xxxxxxxxx> wrote: > --- a/tools/kvm/builtin-run.c > +++ b/tools/kvm/builtin-run.c > @@ -87,9 +87,12 @@ static bool sdl; > static bool balloon; > static bool using_rootfs; > static bool custom_rootfs; > +static bool no_net; > extern bool ioport_debug; > extern int active_console; > extern int debug_iodelay; > +struct virtio_net_parameters *net_params; > +int num_net_devices; Just a stylistic nit-pick: this variable definition section looks pretty ugly. Those externs should be in a header and in any case different types of lines should generally not be mixed without at least a newline between them. > +static int set_net_param(struct virtio_net_parameters *p, const char *param, > + const char *val) Naming nit: 'struct virtio_net_params' is shorter by 4 chars and just as expressive. > + char *buf, *cmd = NULL, *cur = NULL; > + bool on_cmd = true; > + > + if (arg) { > + buf = strdup(arg); > + if (buf == NULL) > + die("Failed allocating new net buffer"); > + cur = strtok(buf, ",="); > + } > + > + p = (struct virtio_net_parameters) { > + .guest_ip = DEFAULT_GUEST_ADDR, > + .host_ip = DEFAULT_HOST_ADDR, > + .script = DEFAULT_SCRIPT, > + .mode = NET_MODE_TAP, > + }; > + > + str_to_mac(DEFAULT_GUEST_MAC, p.guest_mac); > + p.guest_mac[5] += num_net_devices; > + > + while (cur) { > + if (on_cmd) { > + cmd = cur; > + } else { > + if (set_net_param(&p, cmd, cur) < 0) > + goto done; > + } > + on_cmd = !on_cmd; > + > + cur = strtok(NULL, ",="); > + }; > + > + num_net_devices++; > + > + net_params = realloc(net_params, num_net_devices * sizeof(*net_params)); > + if (net_params == NULL) > + die("Failed adding new network device"); > + > + net_params[num_net_devices - 1] = p; > + > +done: > + return 0; > +} Isn't 'buf' leaked here? Patch looks good otherwise. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html