| Message ID | 20191226081521.29652-1-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | nbd/server: improve nbd_negotiate_send_rep_list | expand |
On 12/26/19 2:15 AM, Vladimir Sementsov-Ogievskiy wrote: > Don't try to write zero-lenght strings. length > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > nbd/server.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/nbd/server.c b/nbd/server.c > index 24ebc1a805..28a915f5a2 100644 > --- a/nbd/server.c > +++ b/nbd/server.c > @@ -392,14 +392,18 @@ static int nbd_negotiate_send_rep_list(NBDClient *client, NBDExport *exp, > return -EINVAL; > } > > - if (nbd_write(ioc, name, name_len, errp) < 0) { > - error_prepend(errp, "write failed (name buffer): "); > - return -EINVAL; > + if (name_len > 0) { > + if (nbd_write(ioc, name, name_len, errp) < 0) { What's the rationale for this change? nbd_write() should be a no-op for a zero length write, at which point this is a micro-optimization (fewer CPU cycles, but no semantic change).
08.01.2020 1:01, Eric Blake wrote: > On 12/26/19 2:15 AM, Vladimir Sementsov-Ogievskiy wrote: >> Don't try to write zero-lenght strings. > > length > >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> nbd/server.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/nbd/server.c b/nbd/server.c >> index 24ebc1a805..28a915f5a2 100644 >> --- a/nbd/server.c >> +++ b/nbd/server.c >> @@ -392,14 +392,18 @@ static int nbd_negotiate_send_rep_list(NBDClient *client, NBDExport *exp, >> return -EINVAL; >> } >> - if (nbd_write(ioc, name, name_len, errp) < 0) { >> - error_prepend(errp, "write failed (name buffer): "); >> - return -EINVAL; >> + if (name_len > 0) { >> + if (nbd_write(ioc, name, name_len, errp) < 0) { > > What's the rationale for this change? nbd_write() should be a no-op I looked through the code and it seems for me that nobody does this check, so it would not be a no-op, at least it would be extra syscall.. > for a zero length write, at which point this is a micro-optimization (fewer CPU cycles, but no semantic change). >
diff --git a/nbd/server.c b/nbd/server.c index 24ebc1a805..28a915f5a2 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -392,14 +392,18 @@ static int nbd_negotiate_send_rep_list(NBDClient *client, NBDExport *exp, return -EINVAL; } - if (nbd_write(ioc, name, name_len, errp) < 0) { - error_prepend(errp, "write failed (name buffer): "); - return -EINVAL; + if (name_len > 0) { + if (nbd_write(ioc, name, name_len, errp) < 0) { + error_prepend(errp, "write failed (name buffer): "); + return -EINVAL; + } } - if (nbd_write(ioc, desc, desc_len, errp) < 0) { - error_prepend(errp, "write failed (description buffer): "); - return -EINVAL; + if (desc_len > 0) { + if (nbd_write(ioc, desc, desc_len, errp) < 0) { + error_prepend(errp, "write failed (description buffer): "); + return -EINVAL; + } } return 0;
Don't try to write zero-lenght strings. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- nbd/server.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)