* Philippe Mathieu-Daudé ([email protected]) wrote: > Hi David, > > On 09/08/2017 12:53 PM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <[email protected]> > > > > e.g. > > ./x86_64-softmmu/qemu-system-x86_64 -nographic -netdev > > 'user,id=vnet,hostfwd=:555.0.0.0:0-:22' > > qemu-system-x86_64: -netdev user,id=vnet,hostfwd=:555.0.0.0:0-:22: Invalid > > host forwarding rule ':555.0.0.0:0-:22' (Bad host address) > > > > Signed-off-by: Dr. David Alan Gilbert <[email protected]> > > --- > > net/slirp.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/net/slirp.c b/net/slirp.c > > index 01ed21c006..d87664d42e 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -496,9 +496,11 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, > > char buf[256]; > > int is_udp; > > char *end; > > + const char *fail_reason = ""; > > Isn't it better not initialize this? So if one add a new failed syntax case > the build with abort with -Werror=uninitialized
I never quite trust compilers to spot it or not-moan even though every route to failure will have set it. > Anyway: > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Thanks. Dave > > > p = redir_str; > > if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > > + fail_reason = "No : separators"; > > goto fail_syntax; > > } > > if (!strcmp(buf, "tcp") || buf[0] == '\0') { > > @@ -506,35 +508,43 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, > > } else if (!strcmp(buf, "udp")) { > > is_udp = 1; > > } else { > > + fail_reason = "Bad protocol name"; > > goto fail_syntax; > > } > > if (!legacy_format) { > > if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > > + fail_reason = "Missing : separator"; > > goto fail_syntax; > > } > > if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) { > > + fail_reason = "Bad host address"; > > goto fail_syntax; > > } > > } > > if (get_str_sep(buf, sizeof(buf), &p, legacy_format ? ':' : '-') < 0) > > { > > + fail_reason = "Bad host port separator"; > > goto fail_syntax; > > } > > host_port = strtol(buf, &end, 0); > > if (*end != '\0' || host_port < 0 || host_port > 65535) { > > + fail_reason = "Bad host port"; > > goto fail_syntax; > > } > > if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { > > + fail_reason = "Missing guest address"; > > goto fail_syntax; > > } > > if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) { > > + fail_reason = "Bad guest address"; > > goto fail_syntax; > > } > > guest_port = strtol(p, &end, 0); > > if (*end != '\0' || guest_port < 1 || guest_port > 65535) { > > + fail_reason = "Bad guest port"; > > goto fail_syntax; > > } > > @@ -547,7 +557,8 @@ static int slirp_hostfwd(SlirpState *s, const char > > *redir_str, > > return 0; > > fail_syntax: > > - error_setg(errp, "Invalid host forwarding rule '%s'", redir_str); > > + error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str, > > + fail_reason); > > return -1; > > } > > -- Dr. David Alan Gilbert / [email protected] / Manchester, UK
