[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Bug-gnulib] Re: getaddrinfo
From: |
Simon Josefsson |
Subject: |
[Bug-gnulib] Re: getaddrinfo |
Date: |
Tue, 09 Nov 2004 01:26:54 +0100 |
User-agent: |
Gnus/5.110003 (No Gnus v0.3) Emacs/21.3.50 (gnu/linux) |
Paul Eggert <address@hidden> writes:
> Simon Josefsson <address@hidden> writes:
>
>> +# define EAI_BADFLAGS -1 /* Invalid value for `ai_flags' field.
>> */
>
> "-1" should be parenthesized here, and similarly for the other macros.
>
>> + if (hints &&
>> + hints->ai_protocol != SOCK_STREAM &&
>> + hints->ai_protocol != SOCK_DGRAM)
>> + /* FIXME: Support other protocols. */
>> + return EAI_SERVICE; /* FIXME: Better return code? */
>> ...
>> + if (servname)
>> + {
>> + /* FIXME: Use getservbyname_r if available. */
>> + se = getservbyname (servname,
>> + hints->ai_protocol == SOCK_DGRAM ? "udp" : "tcp");
>> + if (!se)
>> + return EAI_SERVICE;
>> + }
>
> This dumps core if (servname && !hints).
Oops, now it reads:
if (servname)
{
const char *proto =
(hints && hints->ai_protocol == SOCK_DGRAM) ? "udp" : "tcp";
/* FIXME: Use getservbyname_r if available. */
se = getservbyname (servname, proto);
> Also, I don't see why the code cares about hints->ai_protocol when
> !servname.
Shouldn't it do that? The code doesn't care much about
hints->ai_protocol now, except checking for udp and tcp. Do you want
to remove that check? I think the code should ideally care about
ai_protocol more, not less.
> freeaddrinfo has a memory leak: it doesn't free the struct sockaddr_in
> or struct sockaddr_in6 structure allocated by getaddrinfo. To fix
> this I would modify getaddrinfo to invoke malloc just once, to
> allocate all the memory that it needs, rather than invoking it twice.
You mean the addrinfo structs should be allocated as malloc (sizeof
(*res) + sizeof (struct sockaddr_in)) and make the ai_addr pointer
point to after the addrinfo? Sounds doable.
>> +const char *
>> +gai_strerror (int code)
>> +{
>> + size_t i;
>> + for (i = 0; i < sizeof (values) / sizeof (values[0]); ++i)
>> + if (values[i].code == code)
>> + return _(values[i].msg);
>
> A small point: since you specify the encoding for "code" you could
> make gai_strerror an O(1) operation (by using "code" as an index
> directly into a table) rather than O(N).
Yes.
OTOH, if later on, some system happen to define EAI_*, but is missing
the getaddrinfo implementation, it might be better to use the system's
EAI_* values inside gnulib's getaddrinfo. The #define EAI's should
then be wrapped around HAVE_DECL_EAI_FOO etc. In that case, the O(N)
approach is probably required. Also, it can be error prone to keep
the error array and the #define's in sync, especially since "code" is
typically negative, so it can be easy to do a manual +-1 error when
keeping this in sync (OK, not a strong argument).
OTTH, if some system define only some of the POSIX EAI_* errors, it
will be complicated to map the remaining errors into unique values.
So it might be better to do #undef EAI_AGAIN and define the errors
into some known values.
Finally, perhaps a better approach: we should use gai_strerror.c from
glibc CVS instead. I copied the above code from that file, but
keeping the files synchronized seem better. Then if glibc is improved
to do O(1), so will gnulib.
Thanks.
- [Bug-gnulib] Re: getaddrinfo, Simon Josefsson, 2004/11/08
- Re: [Bug-gnulib] Re: getaddrinfo, Paul Eggert, 2004/11/08
- [Bug-gnulib] Re: getaddrinfo,
Simon Josefsson <=
- Re: [Bug-gnulib] Re: getaddrinfo, Bruno Haible, 2004/11/09
- [Bug-gnulib] Re: getaddrinfo, Simon Josefsson, 2004/11/09
- [Bug-gnulib] Re: getaddrinfo, Bruno Haible, 2004/11/09
- [Bug-gnulib] Re: getaddrinfo, Simon Josefsson, 2004/11/09
- Re: [Bug-gnulib] Re: getaddrinfo, Bruno Haible, 2004/11/09
- [Bug-gnulib] Re: getaddrinfo, Simon Josefsson, 2004/11/09
- Re: [Bug-gnulib] Re: getaddrinfo, Paul Eggert, 2004/11/09
- Re: [Bug-gnulib] Re: getaddrinfo, Paul Eggert, 2004/11/09
- Re: [Bug-gnulib] Re: getaddrinfo, Bruno Haible, 2004/11/10
- [Bug-gnulib] Re: getaddrinfo, Simon Josefsson, 2004/11/10