* Sasha Levin <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
