diff mbox series

[3/5] net/tap: tap_set_sndbuf(): return status

Message ID 20201221190609.93768-4-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series net/tap: some fixes and refactorings | expand

Commit Message

Vladimir Sementsov-Ogievskiy Dec. 21, 2020, 7:06 p.m. UTC 
It's recommended to return a value indicating success / failure for
functions with errp parameter (see include/qapi/error.h). Let's update
tap_set_sndbuf().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 net/tap_int.h   | 2 +-
 net/tap-linux.c | 5 ++++-
 net/tap-stub.c  | 2 +-
 net/tap.c       | 6 +++---
 4 files changed, 9 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 21, 2020, 8:12 p.m. UTC | #1 
On 12/21/20 8:06 PM, Vladimir Sementsov-Ogievskiy wrote:
> It's recommended to return a value indicating success / failure for
> functions with errp parameter (see include/qapi/error.h). Let's update
> tap_set_sndbuf().

For simple "success/failure" a bool type is enough.

Preferably using bool type:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  net/tap_int.h   | 2 +-
>  net/tap-linux.c | 5 ++++-
>  net/tap-stub.c  | 2 +-
>  net/tap.c       | 6 +++---
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/net/tap_int.h b/net/tap_int.h
> index 225a49ea48..57567b9f24 100644
> --- a/net/tap_int.h
> +++ b/net/tap_int.h
> @@ -33,7 +33,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>  
>  ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
>  
> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
>  int tap_probe_vnet_hdr(int fd, Error **errp);
>  int tap_probe_vnet_hdr_len(int fd, int len);
>  int tap_probe_has_ufo(int fd);
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index b0635e9e32..c51bcdc2a3 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -130,7 +130,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>   */
>  #define TAP_DEFAULT_SNDBUF 0
>  
> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>  {
>      int sndbuf;
>  
> @@ -144,7 +144,10 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>  
>      if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
>          error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
> +        return -1;
>      }
> +
> +    return 0;
>  }
>  
>  int tap_probe_vnet_hdr(int fd, Error **errp)
> diff --git a/net/tap-stub.c b/net/tap-stub.c
> index 6fa130758b..473d5e4afe 100644
> --- a/net/tap-stub.c
> +++ b/net/tap-stub.c
> @@ -26,7 +26,7 @@
>  #include "qapi/error.h"
>  #include "tap_int.h"
>  
> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>  {
>  }
>  
> diff --git a/net/tap.c b/net/tap.c
> index 75b01d54ee..33d749c7b6 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -661,10 +661,10 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>      Error *err = NULL;
>      TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
>      int vhostfd;
> +    int ret;
>  
> -    tap_set_sndbuf(s->fd, tap, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> +    ret = tap_set_sndbuf(s->fd, tap, errp);
> +    if (ret < 0) {
>          return;
>      }
>  
>
Vladimir Sementsov-Ogievskiy Dec. 22, 2020, 9:50 a.m. UTC | #2 
21.12.2020 23:12, Philippe Mathieu-Daudé wrote:
> On 12/21/20 8:06 PM, Vladimir Sementsov-Ogievskiy wrote:
>> It's recommended to return a value indicating success / failure for
>> functions with errp parameter (see include/qapi/error.h). Let's update
>> tap_set_sndbuf().
> 
> For simple "success/failure" a bool type is enough.
> 
> Preferably using bool type:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

0/-1 return status is highly used in net/tap, so I decided to follow it.

> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   net/tap_int.h   | 2 +-
>>   net/tap-linux.c | 5 ++++-
>>   net/tap-stub.c  | 2 +-
>>   net/tap.c       | 6 +++---
>>   4 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/tap_int.h b/net/tap_int.h
>> index 225a49ea48..57567b9f24 100644
>> --- a/net/tap_int.h
>> +++ b/net/tap_int.h
>> @@ -33,7 +33,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>   
>>   ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
>>   
>> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
>> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
>>   int tap_probe_vnet_hdr(int fd, Error **errp);
>>   int tap_probe_vnet_hdr_len(int fd, int len);
>>   int tap_probe_has_ufo(int fd);
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index b0635e9e32..c51bcdc2a3 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -130,7 +130,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>    */
>>   #define TAP_DEFAULT_SNDBUF 0
>>   
>> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>>   {
>>       int sndbuf;
>>   
>> @@ -144,7 +144,10 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>>   
>>       if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
>>           error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
>> +        return -1;
>>       }
>> +
>> +    return 0;
>>   }
>>   
>>   int tap_probe_vnet_hdr(int fd, Error **errp)
>> diff --git a/net/tap-stub.c b/net/tap-stub.c
>> index 6fa130758b..473d5e4afe 100644
>> --- a/net/tap-stub.c
>> +++ b/net/tap-stub.c
>> @@ -26,7 +26,7 @@
>>   #include "qapi/error.h"
>>   #include "tap_int.h"
>>   
>> -void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>> +int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
>>   {
>>   }
>>   
>> diff --git a/net/tap.c b/net/tap.c
>> index 75b01d54ee..33d749c7b6 100644
>> --- a/net/tap.c
>> +++ b/net/tap.c
>> @@ -661,10 +661,10 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>>       Error *err = NULL;
>>       TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
>>       int vhostfd;
>> +    int ret;
>>   
>> -    tap_set_sndbuf(s->fd, tap, &err);
>> -    if (err) {
>> -        error_propagate(errp, err);
>> +    ret = tap_set_sndbuf(s->fd, tap, errp);
>> +    if (ret < 0) {
>>           return;
>>       }
>>   
>>
>
diff mbox series

Patch

diff --git a/net/tap_int.h b/net/tap_int.h
index 225a49ea48..57567b9f24 100644
--- a/net/tap_int.h
+++ b/net/tap_int.h
@@ -33,7 +33,7 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 
 ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen);
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
+int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
 int tap_probe_vnet_hdr(int fd, Error **errp);
 int tap_probe_vnet_hdr_len(int fd, int len);
 int tap_probe_has_ufo(int fd);
diff --git a/net/tap-linux.c b/net/tap-linux.c
index b0635e9e32..c51bcdc2a3 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -130,7 +130,7 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
  */
 #define TAP_DEFAULT_SNDBUF 0
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
 {
     int sndbuf;
 
@@ -144,7 +144,10 @@  void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
 
     if (ioctl(fd, TUNSETSNDBUF, &sndbuf) == -1 && tap->has_sndbuf) {
         error_setg_errno(errp, errno, "TUNSETSNDBUF ioctl failed");
+        return -1;
     }
+
+    return 0;
 }
 
 int tap_probe_vnet_hdr(int fd, Error **errp)
diff --git a/net/tap-stub.c b/net/tap-stub.c
index 6fa130758b..473d5e4afe 100644
--- a/net/tap-stub.c
+++ b/net/tap-stub.c
@@ -26,7 +26,7 @@ 
 #include "qapi/error.h"
 #include "tap_int.h"
 
-void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
+int tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp)
 {
 }
 
diff --git a/net/tap.c b/net/tap.c
index 75b01d54ee..33d749c7b6 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -661,10 +661,10 @@  static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
     Error *err = NULL;
     TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
     int vhostfd;
+    int ret;
 
-    tap_set_sndbuf(s->fd, tap, &err);
-    if (err) {
-        error_propagate(errp, err);
+    ret = tap_set_sndbuf(s->fd, tap, errp);
+    if (ret < 0) {
         return;
     }