On 12/6/18 8:18 AM, Vladimir Sementsov-Ogievskiy wrote:
01.12.2018 1:03, Eric Blake wrote:
Add some parameters to make this function reusable in upcoming
export listing, where we will want to capture the name and
description rather than compare against a user-supplied name.
No change in semantics to the existing caller.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
nbd/client.c | 66 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 19 deletions(-)
@@ -290,30 +298,49 @@ static int nbd_receive_list(QIOChannel *ioc, const char
*want, bool *match,
nbd_send_opt_abort(ioc);
return -1;
}
- if (namelen != strlen(want)) {
- if (nbd_drop(ioc, len, errp) < 0) {
- error_prepend(errp,
- "failed to skip export name with wrong length: ");
- nbd_send_opt_abort(ioc);
- return -1;
+ if (want) {
+ if (namelen != strlen(want)) {
+ if (nbd_drop(ioc, len, errp) < 0) {
+ error_prepend(errp,
+ "failed to skip export name with wrong length:
");
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
+ return 1;
}
- return 1;
+ assert(namelen < sizeof(array));
+ } else {
+ assert(nameout);
this assert looks a bit redundant, if nameout is 0, next line will abort not
less clearly
+ *nameout = name = g_new(char, namelen + 1);
We should check namelen <= NBD_MAX_NAME_SIZE before it, I think.
Why? We already validated that the overall option is not too large, and
even if the resulting name from the server is larger than
NBD_MAX_NAME_SIZE, it's still nice to report that name to the client for
'qemu-nbd --list'. And these days, we only call nbd_receive_list if a
server lacked NBD_OPT_GO, which is getting rarer and rarer, so
micro-optimizing a rare case to avoid a large malloc isn't going to make
a noticeable difference.
}
- assert(namelen < sizeof(name));
if (nbd_read(ioc, name, namelen, errp) < 0) {
error_prepend(errp, "failed to read export name: ");
nbd_send_opt_abort(ioc);
+ if (!want) {
+ free(name);
g_free
+ }
return -1;
}
name[namelen] = '\0';
len -= namelen;
- if (nbd_drop(ioc, len, errp) < 0) {
+ if (!want) {
+ assert(description);
NBD_MAX_NAME_SIZE
The description is not bound by NBD_MAX_NAME_SIZE. The NBD protocol
DOES document a maximum string size (4k), but we are (so far) not
actually honoring that limit (our choice for NBD_MAX_NAME_SIZE is 256,
which is smaller than the NBD protocol limit).
+ *description = g_new(char, len + 1);
+ if (nbd_read(ioc, *description, len, errp) < 0) {
+ error_prepend(errp, "failed to read export description: ");
+ nbd_send_opt_abort(ioc);
+ free(name);
+ free(*description);
g_free
+ return -1;
+ }
+ (*description)[len] = '\0';
+ } else if (nbd_drop(ioc, len, errp) < 0) {
error_prepend(errp, "failed to read export description: ");
nbd_send_opt_abort(ioc);
return -1;
}
- if (!strcmp(name, want)) {
+ if (want && !strcmp(name, want)) {
*match = true;
}
return 1;
one more thing: on fail path, you finally fill output name and description
with freed pointers. I'd prefer to keep them unchanged in this case, however,
it's a matter of taste.
Okay, I'll try and be more careful in v2 about not altering the callers
pointers on failure.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
