Re: [Xen-devel] [PATCH 1/5] libxl: add PV sound device

[Oleksandr Grytsov <al1img@xxxxxxxxx>]

On Mon, Oct 30, 2017 at 7:39 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:

On Mon, Oct 02, 2017 at 12:49:20PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>
> Add PV sound device described in sndif.h
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>

[...]
>
>  libxl__console_backend = Enumeration("console_backend", [
> diff --git a/tools/libxl/libxl_vsnd.c b/tools/libxl/libxl_vsnd.c
> new file mode 100644
> index 0000000..26885f9
> --- /dev/null
> +++ b/tools/libxl/libxl_vsnd.c
> @@ -0,0 +1,307 @@
> +/*
> + * Copyright (C) 2016 EPAM Systems Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_internal.h"
> +#include "xen/io/sndif.h"
> +

Use <xen/io/sndif.h> -- this is not a local header.

> +
> +static unsigned int libxl__rates_to_str_vsnd(char *str, uint32_t *sample_rates,
> +                                             int num_sample_rates)
> +{
> +    unsigned int len;
> +    int i;
> +
> +    len = 0;
> +
> +    if (num_sample_rates == 0) {
> +        return len;
> +    }

Coding style.

> +
> +    for (i = 0; i < num_sample_rates - 1; i++) {
> +        if (str) {
> +            len += sprintf(&str[len], "%u,", sample_rates[i]);

libxl__sprintf(NOGC, ...)

I need to increment len here. So libxl__sprintf is not suitable.

 

> +        } else {
> +            len += snprintf(NULL, 0, "%u,", sample_rates[i]);
> +        }
> +    }
> +
> +    if (str) {
> +        len += sprintf(&str[len], "%u", sample_rates[i]);
> +    } else {
> +        len += snprintf(NULL, 0, "%u", sample_rates[i]);
> +    }
> +
> +    return len;
> +}
> +
[...]
> +
> +static int libxl__set_params_vsnd(libxl__gc *gc, char *path,
> +                                  libxl_vsnd_params *params, flexarray_t *front)
> +{
> +    char *buffer;
> +    int len;
> +    int rc;
> +
> +    if (params->sample_rates) {
> +        // calculate required string size;

Coding style.

> +        len = libxl__rates_to_str_vsnd(NULL, params->sample_rates,
> +                                       params->num_sample_rates);
> +
> +        if (len) {
> +            buffer = libxl__malloc(gc, len + 1);
> +
> +            libxl__rates_to_str_vsnd(buffer, params->sample_rates,
> +                                     params->num_sample_rates);
> +            rc = flexarray_append_pair(front,
> +                                       GCSPRINTF("%s"XENSND_FIELD_SAMPLE_RATES,
> +                                                 path), buffer);
> +            if (rc) return rc;

goto out please.

Please fix these coding style issues throughout this series.j

--

Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel