Re: [Qemu-devel] [PATCH v2 4/6] nbd/server: Add va_list form of nbd_negotiate_send_rep_err()

[Vladimir Sementsov-Ogievskiy]

11.01.2018 02:08, Eric Blake wrote:
> This will be useful for the next patch.
>
> Based on a patch by Vladimir Sementsov-Ogievskiy
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
>   nbd/server.c | 24 ++++++++++++++++++------
>   1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index 9943a911c3..d23bc2918a 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -186,18 +186,15 @@ static int nbd_negotiate_send_rep(NBDClient *client, 
> uint32_t type,
>
>   /* Send an error reply.
>    * Return -errno on error, 0 on success. */
> -static int GCC_FMT_ATTR(4, 5)
> -nbd_negotiate_send_rep_err(NBDClient *client, uint32_t type,
> -                           Error **errp, const char *fmt, ...)
> +static int GCC_FMT_ATTR(4, 0)
> +nbd_negotiate_send_rep_verr(NBDClient *client, uint32_t type,
> +                            Error **errp, const char *fmt, va_list va)

Hmm you placed fmt and va after errp. Previously we discussed one 
exclusion from "errp should be last" -
"...", variable number of arguments. So, it is new exclusion (or I 
missed something?).. Looks good for me,
anyway, as it corresponds to "errp, fmt, ..." notation.

Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

>   {
> -    va_list va;
>       char *msg;
>       int ret;
>       size_t len;
>
> -    va_start(va, fmt);
>       msg = g_strdup_vprintf(fmt, va);
> -    va_end(va);
>       len = strlen(msg);
>       assert(len < 4096);
>       trace_nbd_negotiate_send_rep_err(msg);
> @@ -217,6 +214,21 @@ out:
>       return ret;
>   }
>
> +/* Send an error reply.
> + * Return -errno on error, 0 on success. */
> +static int GCC_FMT_ATTR(4, 5)
> +nbd_negotiate_send_rep_err(NBDClient *client, uint32_t type,
> +                           Error **errp, const char *fmt, ...)
> +{
> +    va_list va;
> +    int ret;
> +
> +    va_start(va, fmt);
> +    ret = nbd_negotiate_send_rep_verr(client, type, errp, fmt, va);
> +    va_end(va);
> +    return ret;
> +}
> +
>   /* Send a single NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
>    * Return -errno on error, 0 on success. */
>   static int nbd_negotiate_send_rep_list(NBDClient *client, NBDExport *exp,


--
Best regards,
Vladimir