diff mbox series

net/slirp: fix the error message when the prefix len is invalid

Message ID 20190509133737.242548-1-sgarzare@redhat.com (mailing list archive)
State New, archived
Headers show
Series net/slirp: fix the error message when the prefix len is invalid | expand

Commit Message

Stefano Garzarella May 9, 2019, 1:37 p.m. UTC
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(-)

Comments

Marc-André Lureau May 9, 2019, 1:54 p.m. UTC | #1
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
>
>
Markus Armbruster May 9, 2019, 2:54 p.m. UTC | #2
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"?
Stefano Garzarella May 9, 2019, 3:16 p.m. UTC | #3
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
Markus Armbruster May 10, 2019, 5:56 a.m. UTC | #4
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.
Stefano Garzarella May 10, 2019, 7:58 a.m. UTC | #5
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
diff mbox series

Patch

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