Message ID | 20180110230825.18321-5-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Series | NBD server refactoring before BLOCK_STATUS | expand |
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 <eblake@redhat.com> > --- > 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 <vsementsov@virtuozzo.com> > { > - 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,
On 01/11/2018 12:05 PM, Vladimir Sementsov-Ogievskiy wrote: > 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 <eblake@redhat.com> >> --- >> nbd/server.c | 24 ++++++++++++++++++------ >> 1 file changed, 18 insertions(+), 6 deletions(-) >> >> -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. Indeed, it is precisely because I value consistency between 'fmt, ...' and 'fmt, va_list' higher than 'errp last' that I rearranged things from your patch into the form I used. Or worded another way, even though va_list is only one argument in name, I treat it the same as the variable number of arguments that warrants the exception to the errp last rule of thumb. > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Thanks; I think this series is now ready for staging on my NBD queue; although my next pull request won't be until after the weekend.
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) { - 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,
This will be useful for the next patch. Based on a patch by Vladimir Sementsov-Ogievskiy Signed-off-by: Eric Blake <eblake@redhat.com> --- nbd/server.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)