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

[Eric Blake]

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 
>> ---
>>   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 
> 

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

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 
---
  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 


  {
-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

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

[Eric Blake]

This will be useful for the next patch.

Based on a patch by Vladimir Sementsov-Ogievskiy

Signed-off-by: Eric Blake 
---
 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)
 {
-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,
-- 
2.14.3