nbd/server: improve nbd_negotiate_send_rep_list

[PATCH] nbd/server: improve nbd_negotiate_send_rep_list

Posted by Vladimir Sementsov-Ogievskiy 4 years, 4 months ago

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

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

Re: [PATCH] nbd/server: improve nbd_negotiate_send_rep_list

Posted by Eric Blake 4 years, 3 months ago

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

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

Re: [PATCH] nbd/server: improve nbd_negotiate_send_rep_list

Posted by Vladimir Sementsov-Ogievskiy 4 years, 3 months ago

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



-- 
Best regards,
Vladimir