* Sasha Levin <levinsasha...@gmail.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to