[Qemu-devel] [PATCH] slirp: Clean up net_slirp_hostfwd_remove()'s use of get_str_sep()

[Qemu-devel] [PATCH] slirp: Clean up net_slirp_hostfwd_remove()'s use of get_str_sep()

[Markus Armbruster]

get_str_sep() can fail, but net_slirp_hostfwd_remove() doesn't check.
Works, because it initializes buf[] to "", which get_str_sep() doesn't
touch when it fails.  Coverity doesn't like it, and neither do I.

Change it to work exactly like slirp_hostfwd().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/slirp.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index c6cda5d..6646ecb 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -305,7 +305,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
     struct in_addr host_addr = { .s_addr = INADDR_ANY };
     int host_port;
-    char buf[256] = "";
+    char buf[256];
     const char *src_str, *p;
     SlirpState *s;
     int is_udp = 0;
@@ -325,11 +325,10 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    if (!src_str || !src_str[0])
-        goto fail_syntax;
-
     p = src_str;
-    get_str_sep(buf, sizeof(buf), &p, ':');
+    if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+        goto fail_syntax;
+    }
 
     if (!strcmp(buf, "tcp") || buf[0] == '\0') {
         is_udp = 0;
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH] slirp: Clean up net_slirp_hostfwd_remove()'s use of get_str_sep()

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On 2011-11-16 12:45, Markus Armbruster wrote:
> get_str_sep() can fail, but net_slirp_hostfwd_remove() doesn't check.
> Works, because it initializes buf[] to "", which get_str_sep() doesn't
> touch when it fails.  Coverity doesn't like it, and neither do I.
> 
> Change it to work exactly like slirp_hostfwd().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Acked-by: Jan Kiszka <jan.kiszka@siemens.com>

Stefan, can you take this? My slirp queue not fully "operative" ATM down
here in Brazil.

Thanks,
Jan

> ---
>  net/slirp.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index c6cda5d..6646ecb 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -305,7 +305,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
>  {
>      struct in_addr host_addr = { .s_addr = INADDR_ANY };
>      int host_port;
> -    char buf[256] = "";
> +    char buf[256];
>      const char *src_str, *p;
>      SlirpState *s;
>      int is_udp = 0;
> @@ -325,11 +325,10 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict)
>          return;
>      }
>  
> -    if (!src_str || !src_str[0])
> -        goto fail_syntax;
> -
>      p = src_str;
> -    get_str_sep(buf, sizeof(buf), &p, ':');
> +    if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
> +        goto fail_syntax;
> +    }
>  
>      if (!strcmp(buf, "tcp") || buf[0] == '\0') {
>          is_udp = 0;



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] slirp: Clean up net_slirp_hostfwd_remove()'s use of get_str_sep()

[Stefan Hajnoczi]

On Wed, Nov 16, 2011 at 03:45:59PM +0100, Markus Armbruster wrote:
> get_str_sep() can fail, but net_slirp_hostfwd_remove() doesn't check.
> Works, because it initializes buf[] to "", which get_str_sep() doesn't
> touch when it fails.  Coverity doesn't like it, and neither do I.
> 
> Change it to work exactly like slirp_hostfwd().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  net/slirp.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)

Thanks, applied to the trivial patches -next tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches-next

Stefan