07.02.2017 19:32, Eric Blake wrote:
On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:
Split out nbd_receive_simple_option to be reused for structured reply
option.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
nbd/client.c | 54 +++++++++++++++++++++++++++++++++++-------------------
nbd/nbd-internal.h | 14 ++++++++++++++
2 files changed, 49 insertions(+), 19 deletions(-)
+++ b/nbd/nbd-internal.h
@@ -96,6 +96,20 @@
#define NBD_ENOSPC 28
#define NBD_ESHUTDOWN 108
+static inline const char *nbd_opt_name(int opt)
+{
+ switch (opt) {
+ case NBD_OPT_EXPORT_NAME: return "export_name";
Does this really get past checkpatch?
+ case NBD_OPT_ABORT: return "abort";
+ case NBD_OPT_LIST: return "list";
+ case NBD_OPT_PEEK_EXPORT: return "peek_export";
+ case NBD_OPT_STARTTLS: return "tls";
Why just 'tls' instead of 'starttls'?
+ case NBD_OPT_STRUCTURED_REPLY: return "structured_reply";
+ }
+
+ return "<unknown option>";
Can you please consider making this include the %d representation of the
unknown option; perhaps by snprintf'ing into static storage? While it
Hmm.. The caller should free it in this case. Currently, the
get_opt_name is called only on error path or if tracing enabled. How to
achieve this with variable string return value not complicating the code
a lot? When there is unknown message there should be other mechanism to
inform user
is unlikely that a well-behaved server will respond to a client with an
option the client doesn't recognize, it is much more likely that this
reverse lookup function will be used in a server to respond to an
unknown option from a client.
In fact, I might have split this into two patches: one providing
nbd_opt_name() and using it throughout the code base where appropriate,
and the other refactoring starttls in the client.
I'm not sure if the reverse lookup function needs to be inline in the
header; it could reasonably live in nbd/common.c, particularly if you
are going to take my advice to have it format a message for unknown values.
--
Best regards,
Vladimir
