Add a missing parentheses at the end of the error message,
when we have an invalid prefix len.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/slirp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/slirp.c b/net/slirp.c
index 95934fb36d..0f4ae0abc0 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     }
     if (vprefix6_len < 0 || vprefix6_len > 126) {
         error_setg(errp,
-                   "Invalid prefix provided (prefix len must be in range 0-126");
+                   "Invalid prefix provided "
+                   "(prefix len must be in range 0-126)");
         return -1;
     }
 
-- 
2.20.1

Stefano Garzarella <sgarzare@redhat.com> writes:

> Add a missing parentheses at the end of the error message,
> when we have an invalid prefix len.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/slirp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 95934fb36d..0f4ae0abc0 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      }
>      if (vprefix6_len < 0 || vprefix6_len > 126) {
>          error_setg(errp,
> -                   "Invalid prefix provided (prefix len must be in range 0-126");
> +                   "Invalid prefix provided "
> +                   "(prefix len must be in range 0-126)");
>          return -1;
>      }

Preexisting: the error message fails to identify the offending
parameter.  The user needs to make the connection to "ipv6-prefixlen"
based on the fact that the only parameters with "prefix" in name or
description are "ipv6-prefix" and "ipv6-prefixlen", and only the latter
is a length.

What about "Parameter 'ipv6-prefixlen' expects a length below 127", or
"Parameter 'ipv6-prefixlen' expects a value between 0 and 126"?

On Thu, May 09, 2019 at 04:54:35PM +0200, Markus Armbruster wrote:
> Stefano Garzarella <sgarzare@redhat.com> writes:
> 
> > Add a missing parentheses at the end of the error message,
> > when we have an invalid prefix len.
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  net/slirp.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/slirp.c b/net/slirp.c
> > index 95934fb36d..0f4ae0abc0 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
> >      }
> >      if (vprefix6_len < 0 || vprefix6_len > 126) {
> >          error_setg(errp,
> > -                   "Invalid prefix provided (prefix len must be in range 0-126");
> > +                   "Invalid prefix provided "
> > +                   "(prefix len must be in range 0-126)");
> >          return -1;
> >      }
> 
> Preexisting: the error message fails to identify the offending
> parameter.  The user needs to make the connection to "ipv6-prefixlen"
> based on the fact that the only parameters with "prefix" in name or
> description are "ipv6-prefix" and "ipv6-prefixlen", and only the latter
> is a length.
> 
> What about "Parameter 'ipv6-prefixlen' expects a length below 127", or
> "Parameter 'ipv6-prefixlen' expects a value between 0 and 126"?

"Parameter 'ipv6-prefixlen' expects a value between 0 and 126" should be
fine.

Otherwise, since other errors didn't refer to the parameter name, we can
simply add IPv6 in this way:
"Invalid IPv6 prefix provided (IPv6 prefix len must be between 0 and 126)"

But I'm fine also with your proposal.

Thanks,
Stefano

Stefano Garzarella <sgarzare@redhat.com> writes:

> On Thu, May 09, 2019 at 04:54:35PM +0200, Markus Armbruster wrote:
>> Stefano Garzarella <sgarzare@redhat.com> writes:
>> 
>> > Add a missing parentheses at the end of the error message,
>> > when we have an invalid prefix len.
>> >
>> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > ---
>> >  net/slirp.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/net/slirp.c b/net/slirp.c
>> > index 95934fb36d..0f4ae0abc0 100644
>> > --- a/net/slirp.c
>> > +++ b/net/slirp.c
>> > @@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>> >      }
>> >      if (vprefix6_len < 0 || vprefix6_len > 126) {
>> >          error_setg(errp,
>> > -                   "Invalid prefix provided (prefix len must be in range 0-126");
>> > +                   "Invalid prefix provided "
>> > +                   "(prefix len must be in range 0-126)");
>> >          return -1;
>> >      }
>> 
>> Preexisting: the error message fails to identify the offending
>> parameter.  The user needs to make the connection to "ipv6-prefixlen"
>> based on the fact that the only parameters with "prefix" in name or
>> description are "ipv6-prefix" and "ipv6-prefixlen", and only the latter
>> is a length.
>> 
>> What about "Parameter 'ipv6-prefixlen' expects a length below 127", or
>> "Parameter 'ipv6-prefixlen' expects a value between 0 and 126"?
>
> "Parameter 'ipv6-prefixlen' expects a value between 0 and 126" should be
> fine.
>
> Otherwise, since other errors didn't refer to the parameter name, we can
> simply add IPv6 in this way:
> "Invalid IPv6 prefix provided (IPv6 prefix len must be between 0 and 126)"

"len" is not a word.  Either say "ipv6-prefixlen", or say "IPv6 prefix
length".

> But I'm fine also with your proposal.

It's just a suggestion.  Feel free to leave the error messages
consistently vague (apply your patch as is), improve just this one, or
improve more messages.

On Fri, May 10, 2019 at 07:56:47AM +0200, Markus Armbruster wrote:
> Stefano Garzarella <sgarzare@redhat.com> writes:
> 
> > On Thu, May 09, 2019 at 04:54:35PM +0200, Markus Armbruster wrote:
> >> Stefano Garzarella <sgarzare@redhat.com> writes:
> >> 
> >> > Add a missing parentheses at the end of the error message,
> >> > when we have an invalid prefix len.
> >> >
> >> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >> > ---
> >> >  net/slirp.c | 3 ++-
> >> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/net/slirp.c b/net/slirp.c
> >> > index 95934fb36d..0f4ae0abc0 100644
> >> > --- a/net/slirp.c
> >> > +++ b/net/slirp.c
> >> > @@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
> >> >      }
> >> >      if (vprefix6_len < 0 || vprefix6_len > 126) {
> >> >          error_setg(errp,
> >> > -                   "Invalid prefix provided (prefix len must be in range 0-126");
> >> > +                   "Invalid prefix provided "
> >> > +                   "(prefix len must be in range 0-126)");
> >> >          return -1;
> >> >      }
> >> 
> >> Preexisting: the error message fails to identify the offending
> >> parameter.  The user needs to make the connection to "ipv6-prefixlen"
> >> based on the fact that the only parameters with "prefix" in name or
> >> description are "ipv6-prefix" and "ipv6-prefixlen", and only the latter
> >> is a length.
> >> 
> >> What about "Parameter 'ipv6-prefixlen' expects a length below 127", or
> >> "Parameter 'ipv6-prefixlen' expects a value between 0 and 126"?
> >
> > "Parameter 'ipv6-prefixlen' expects a value between 0 and 126" should be
> > fine.
> >
> > Otherwise, since other errors didn't refer to the parameter name, we can
> > simply add IPv6 in this way:
> > "Invalid IPv6 prefix provided (IPv6 prefix len must be between 0 and 126)"
> 
> "len" is not a word.  Either say "ipv6-prefixlen", or say "IPv6 prefix
> length".
> 
> > But I'm fine also with your proposal.
> 
> It's just a suggestion.  Feel free to leave the error messages
> consistently vague (apply your patch as is), improve just this one, or
> improve more messages.

Your suggestions are very appreciated!

I'll resend this patch fixing this error message and I'll check also
the other messages.

Thanks,
Stefano

On Thu, May 9, 2019 at 3:39 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> Add a missing parentheses at the end of the error message,
> when we have an invalid prefix len.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  net/slirp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 95934fb36d..0f4ae0abc0 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -498,7 +498,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      }
>      if (vprefix6_len < 0 || vprefix6_len > 126) {
>          error_setg(errp,
> -                   "Invalid prefix provided (prefix len must be in range 0-126");
> +                   "Invalid prefix provided "
> +                   "(prefix len must be in range 0-126)");
>          return -1;
>      }
>
> --
> 2.20.1
>
>


-- 
Marc-André Lureau