Re: [Qemu-devel] [PATCH v6 3/4] net/net: Convert parse_host_port() to Error

[Mao Zhongyi]

Hi, Daniel

On 06/28/2017 09:23 PM, Daniel P. Berrange wrote:
> On Wed, Jun 28, 2017 at 09:08:49PM +0800, Mao Zhongyi wrote:
> > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> > index 5c326db..78e2b30 100644
> > --- a/include/qemu/sockets.h
> > +++ b/include/qemu/sockets.h
>
> >          if (qemu_isdigit(buf[0])) {
> > -            if (!inet_aton(buf, &saddr->sin_addr))
> > +            if (!inet_aton(buf, &saddr->sin_addr)) {
> > +                error_setg(errp, "host address '%s' is not a valid "
> > +                           "IPv4 address", buf);
> >                  return -1;
> > +            }
> >          } else {
> > -            if ((he = gethostbyname(buf)) == NULL)
> > +            he = gethostbyname(buf);
> > +            if (he == NULL) {
> > +                error_setg(errp, "can't resolve host address '%s': "
> > +                           "unknown host", buf);
> >                  return - 1;
> > +            }
>
> gethostbyname sets  'h_errno' on failure, so you should pass that
> into error_setg_errno, instead of hardcoding 'unknown host' as a
> message

Yes, 'h_errno' will be seted detailed error message when gethostbyname() failed,
but the value of 'h_errno', as far as I know, can be print by hstrerror(),
herror() and gai_strerror(). I'm not sure 'h_errno' can be parsed correctly
by error_setg_errno(), so I test it like this:

herror("----herror----");
error_report("----hstrerror----%s", hstrerror(h_errno));
error_report("----gai_strerror----%s", gai_strerror(h_errno));
error_setg_errno(errp, h_errno, "can't resolve host address '%s'" ,buf);


result:

----herror----: Unknown host
qemu-system-x86_64: -net socket,listen=hostname:: ----hstrerror----Unknown host
qemu-system-x86_64: -net socket,listen=hostname:: ----gai_strerror----Unknown 
error
qemu-system-x86_64: -net socket,listen=hostname:: can't resolve host address 
'hostname': Operation not permitted


From the results, obviously the error message is different.
error_setg_errno() prints a uncertain message like "Operation not
permitted", I think that the value of 'h_errno' is treated as a
standard error code for errno. Although they have the same value,
the information contained may not be same.

so is it really appropriate to pass 'h_errno' to error_setg_errno()?

Thanks,
Mao


> Regards,
> Daniel