Re: canon-host errors
Derek Price [EMAIL PROTECTED] wrote: I've installed the attached patch. It is almost identical to my previous one, with a few extra portability and typo fixes. 2005-09-12 Derek Price [EMAIL PROTECTED] * modules/canon-host: Add canon-host.h. Depend on getaddrinfo. Make LGPL. * modules/getaddrinfo: Add link to opengroup spec. Depend on strdup. Thanks for doing all of that. There is one small problem. When strdup returns NULL, canon_host_r returns NULL without setting *cherror. Shouldn't that be fixed? I've checked in these cosmetic changes: That file was using an indentation style different from the style used in nearly every other source file. I ran it through GNU indent. Finally, we prefer to use an active voice (rather than passive voice) in comments. So I changed this: /* Returns a malloc'd string containing the canonical hostname associated with HOST, or NULL if a canonical name cannot be determined. On NULL return, if CHERROR is not NULL, *CHERROR will be set to an error code as returned by getaddrinfo(). Error codes from CHERROR may be converted to a string suitable for error messages by ch_strerror_r() or gai_strerror(). to this: /* Return a malloc'd string containing the canonical hostname associated with HOST, or NULL if a canonical name cannot be determined. On NULL return, if CHERROR is not NULL, set *CHERROR to an error code as returned by getaddrinfo(). Use ch_strerror_r() or gai_strerror() to convert a *CHERROR value to a string suitable for error messages. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Jim Meyering wrote:
Derek Price [EMAIL PROTECTED] wrote:
I've installed the attached patch. It is almost identical to my
previous one, with a few extra portability and typo fixes.
2005-09-12 Derek Price [EMAIL PROTECTED]
* modules/canon-host: Add canon-host.h. Depend on getaddrinfo. Make
LGPL.
* modules/getaddrinfo: Add link to opengroup spec. Depend on strdup.
Thanks for doing all of that.
There is one small problem.
When strdup returns NULL, canon_host_r
returns NULL without setting *cherror.
Shouldn't that be fixed?
Yes. Thanks. I've installed the attached patch.
2005-09-13 Derek Price [EMAIL PROTECTED]
* canon-host.c (canon_host_r): Set *cherror on memory allocation
failure.
Reported by Jim Meyering [EMAIL PROTECTED].
I've checked in these cosmetic changes:
Thanks.
That file was using an indentation style different from the style
used in nearly every other source file. I ran it through GNU indent.
Thanks. I meant to do that and forgot.
Finally, we prefer to use an active voice (rather than passive voice)
in comments. So I changed this:
Thanks.
Regards,
Derek
--
Derek R. Price
CVS Solutions Architect
Ximbiot http://ximbiot.com
v: +1 717.579.6168
f: +1 717.234.3125
mailto:[EMAIL PROTECTED]
Index: lib/canon-host.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/canon-host.c,v
retrieving revision 1.20
diff -u -p -r1.20 canon-host.c
--- lib/canon-host.c13 Sep 2005 12:37:48 - 1.20
+++ lib/canon-host.c13 Sep 2005 12:59:11 -
@@ -72,6 +72,8 @@ canon_host_r (char const *host, int *che
if (!status)
{
retval = strdup (res-ai_canonname);
+ if (!retval cherror)
+ *cherror = EAI_MEMORY;
freeaddrinfo (res);
}
else if (cherror)
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
AC 2.59 incompatibility in getaddrinfo.h (was Re: canon-host errors)
Is there any reason I can't just assume gl_GETADDRINFO ran and config.h was included before getaddrinfo.h? The following test is always coming up false on platforms without getaddrinfo (as of AC 2.59, at least, AC_CHECK_FUNCS via AC_REPLACE_FUNCS leaves HAVE_GETADDRINFO undefined when it is not found): # if defined HAVE_GETADDRINFO !HAVE_GETADDRINFO [...decl struct addrinfo many macros...] # endif The possible fixes are fixing gl_GETADDRINFO to #define HAVE_GETADDRINFO 0 when getaddrinfo isn't found or shortening the above test to: # if !HAVE_GETADDRINFO, and I'd rather just simplify the header unless there is good reason to leave it as is, especially as other naive code with GNULIB installed may be already using #ifdef HAVE_GETADDRINFO somewhere. Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price [EMAIL PROTECTED] writes: * modules/getaddrinfo: Add link to opengroup spec. Depend on strdup. Make canon-host require getaddrinfo. * m4/getaddrinfo.m4 (gl_GETADDRINFO): Compile gai_strerror when needed. Return usable errors from canon-host. * lib/getaddrinfo.c: Move include of getaddrinfo.h first to test interface. (getaddrinfo): Add AI_CANONNAME functionality. (freeaddrinfo): Free ai-ai_canonname when set. The getaddrinfo related changes look good to me, thanks for improving the module! ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Jim Meyering wrote:
suitable for gai_strerror. I'll have to extend lib/getaddrinfo.c a
little to fill in ai_canonhost and add the gai_strerror function. Would
that be acceptable?
Good idea. More than `acceptable' :-)
Patch attached. I decided the reverse-lookup which used to be present
in canon-host.c when gethostbyname returned an IP address was the bug since:
- all the documentation on getaddrinfo and canonical names was
quite explicit about reverse lookups *not* being done
- canon_host used to trust getaddrinfo when it was present
- the documentation on gethostbyname also implied that the
hostname would be resolved and a test program on 1 linux
NetBSD 1.6.1 showed that it was.
If we stumble across any systems where gethostbyname returns an IP
address when it is given a real hostname, this can be fixed then.
2005-09-05 Derek Price [EMAIL PROTECTED]
* modules/canon-host: Add canon-host.h. Depend on getaddrinfo. Make
LGPL.
* modules/getaddrinfo: Add link to opengroup spec. Depend on strdup.
Make canon-host require getaddrinfo.
* m4/canon-host.m4 (gl_CANON_HOST): Remove most dependencies.
AC_LIBSOURCE canon-host.h. Call...
(gl_PREREQ_CANON_HOST): ...this new function, which requires
gl_GETADDRINFO.
* m4/getaddrinfo.m4 (gl_GETADDRINFO): Compile gai_strerror when needed.
Return usable errors from canon-host.
* lib/canon-host.h: New file.
* lib/canon-host.c (canon_host): Wrap...
(canon_host_r): ...this new function, which now relies exclusively on
getaddrinfo.
(ch_strerror): New function.
(last_cherror): New global.
* lib/getaddrinfo.c: Move include of getaddrinfo.h first to test
interface.
(getaddrinfo): Add AI_CANONNAME functionality.
(freeaddrinfo): Free ai-ai_canonname when set.
This works on 1 Linux and I am installing in CVS to run it through our
compile farm. If all goes well and no one objects, I'll install it in
GNULIB in a few days.
Cheers,
Derek
--
Derek R. Price
CVS Solutions Architect
Ximbiot http://ximbiot.com
v: +1 717.579.6168
f: +1 717.234.3125
mailto:[EMAIL PROTECTED]
Index: lib/canon-host.c
===
RCS file: /cvsroot/gnulib/gnulib/lib/canon-host.c,v
retrieving revision 1.18
diff -u -p -r1.18 canon-host.c
--- lib/canon-host.c24 Jun 2005 17:30:33 - 1.18
+++ lib/canon-host.c6 Sep 2005 02:46:13 -
@@ -1,9 +1,9 @@
/* Host name canonicalization
- Copyright (C) 1995, 1999, 2000, 2002, 2003, 2004, 2005 Free Software
+ Copyright (C) 2005 Free Software
Foundation, Inc.
- Written by Miles Bader [EMAIL PROTECTED]
+ Written by Derek Price [EMAIL PROTECTED].
This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License as
@@ -23,105 +23,76 @@
# include config.h
#endif
-#include sys/types.h
-#ifdef HAVE_UNISTD_H
-# include unistd.h
-#endif
-#include stdlib.h
-#include string.h
-#ifdef HAVE_NETDB_H
-# include netdb.h
-#endif
-#ifdef HAVE_SYS_SOCKET_H
-# include sys/socket.h
-#endif
-
-#ifdef HAVE_NETINET_IN_H
-# include netinet/in.h
-#endif
-#ifdef HAVE_ARPA_INET_H
-# include arpa/inet.h
-#endif
+#include canon-host.h
+#include getaddrinfo.h
#include strdup.h
-/* Returns the canonical hostname associated with HOST (allocated in a static
- buffer), or NULL if it can't be determined. */
+
+
+/* Store the last error for the single-threaded version of this function. */
+static int last_cherror;
+
+
+
+/* Single-threaded of wrapper for canon_host_r. After a NULL return, error
+ messages may be retrieved via ch_strerror().
+ */
char *
-canon_host (char const *host)
+canon_host (const char *host)
{
- char *h_addr_copy = NULL;
+return canon_host_r (host, last_cherror);
+}
-#if HAVE_GETADDRINFO
- {
-struct addrinfo hint = { 0, };
+
+
+/* Returns a malloc'd string containing the canonical hostname associated with
+ HOST, or NULL if a canonical name cannot be determined. On NULL return, if
+ CHERROR is not NULL, *CHERROR will be set to an error code as returned by
+ getaddrinfo(). Error codes from CHERROR may be converted to a string
+ suitable for error messages by ch_strerror_r() or gai_strerror().
+
+ WARNINGS
+ HOST must be a string representation of a resolvable name for this host.
+ Strings containing an IP address in dotted decimal notation will be
+ returned as-is, without further resolution.
+
+ The use of the word canonical in this context is unfortunate but
+ entrenched. The value returned by this function will be the end result
+ of the resolution of any CNAME chains in the DNS. There may only be one
+ such value for any given hostname, though the actual IP address
+ referenced by this value and the device using that IP address may each
+ actually have any number of such canonical hostnames. See the POSIX
+ getaddrinfo spec
Re: canon-host errors
Derek Price [EMAIL PROTECTED] wrote: Would anyone object to a patch that caused canon-host to output warnings via error (0, ...) when one of the functions it calls fails? getaddrinfo returns errors differently than gethostbyname and gethostbyaddr, making outputting a useful error message upon seeing a simple NULL return from canon_host challenging, to say the least. The only two uses in coreutils are in who.c and pinky.c, so that wouldn't cause much trouble. But if some library code uses canon_host, such a change wouldn't go down as well. I suppose the question is whether any library-like code (that would not welcome the output to stderr) uses canon_host. Personally, I don't mind if you add the code to emit warnings now, as long as you agree to adjust the API later, (e.g., to add a new enum parameter describing the error) if anyone complains. Of course, it'd be better to keep it library-safe. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Jim Meyering wrote: Personally, I don't mind if you add the code to emit warnings now, as long as you agree to adjust the API later, (e.g., to add a new enum parameter describing the error) if anyone complains. Of course, it'd be better to keep it library-safe. Hrm. A new enum parameter would mean duplicating a bunch of potential POSIX error codes from the other three functions. How about if I leave the enum parameter somewhat opaque and provide a canon_host_error (perhaps strcherror is a better name) to interpret it and print the error if desired? I'd really prefer to leave the error as opaque as possible, even to the canon_host strcherror functions. Is there any convenient yet portable way I could merge the two sets of error codes into one range of values? Shifting one set left 16 bits? Maybe the most straightforward way to go about it is simply to return the error as two values, an enum with values like CH_NO_ERROR, CH_EAI_ERROR, CH_HERROR, plus an int which either holds the return code from getaddrinfo or the value of h_error upon return from gethostbyname or gethostbyaddr (and is undefined on success)? Then strcherror canon-host would have protos like: char *strcherror (enum canon_error errtype, int errcode); char *canon_host (char *hostname, enum canon_error *errtype, int *errcode); Callers not interested in the error codes could pass NULL in for errtype errcode. Could probably even add CH_USE_GLOBAL to the enum and keep the last error encountered in a static global for single-threaded apps, but maybe that is overkill? Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Jim Meyering wrote: I like your idea of keeping them separate. How about passing either NULL or the address of a struct containing a member for each error indicator? Okay. Will do. Should I ignore single-threaded apps entirely, keep the error data in a global to simplify for single-threaded apps on NULL, or break the functions into canon_host, canon_host_r, strcherror, strcherror_r? Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price [EMAIL PROTECTED] wrote: Hrm. A new enum parameter would mean duplicating a bunch of potential POSIX error codes from the other three functions. How about if I leave the enum parameter somewhat opaque and provide a canon_host_error (perhaps strcherror is a better name) to interpret it and print the error if desired? I'd really prefer to leave the error as opaque as possible, even to the canon_host strcherror functions. That sounds fine. Is there any convenient yet portable way I could merge the two sets of error codes into one range of values? Shifting one set left 16 bits? Maybe the most straightforward way to go about it is simply to return the error as two values, an enum with values like CH_NO_ERROR, CH_EAI_ERROR, CH_HERROR, plus an int which either holds the return code from getaddrinfo or the value of h_error upon return from gethostbyname or gethostbyaddr (and is undefined on success)? Then strcherror canon-host would have protos like: char *strcherror (enum canon_error errtype, int errcode); char *canon_host (char *hostname, enum canon_error *errtype, int *errcode); Callers not interested in the error codes could pass NULL in for errtype errcode. Could probably even add CH_USE_GLOBAL to the enum and keep the last error encountered in a static global for single-threaded apps, but maybe that is overkill? I like your idea of keeping them separate. How about passing either NULL or the address of a struct containing a member for each error indicator? ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price [EMAIL PROTECTED] wrote: Okay. Will do. Should I ignore single-threaded apps entirely, keep the error data in a global to simplify for single-threaded apps on NULL, or break the functions into canon_host, canon_host_r, strcherror, strcherror_r? I don't thinks it's worthwhile to pander to single-threaded applications for something like this. I'd prefer a new, thread-safe interface. Maybe what we need is a new module, canon-host-r, that contains most of the current canon-host.c. Then canon-host.c could be rewritten as a wrapper around canon_host_r. But hey, if you're implementing it, your opinion counts for a lot :) ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Jim Meyering wrote:
I don't thinks it's worthwhile to pander to single-threaded
applications for something like this.
In this case, pandering isn't very complicated, so I think I'll do it.
Besides, this way, if anyone else is using the canon-host module, they
won't need to update callers after this change.
I'd prefer a new, thread-safe interface.
Maybe what we need is a new module, canon-host-r,
that contains most of the current canon-host.c.
Then canon-host.c could be rewritten as a wrapper
around canon_host_r.
This was about what I was thinking, though I was going to combine
canon_host canon_host_r in the canon-host module and let the caller
decide what to call. How about this API:
struct cherror_s
{
enum {CH_GAI_ERROR, CH_HERROR} errtype;
int errcode;
};
char *canon_host (char const *host);
char *canon_host_r (char const *host, struct cherror_s *cherror);
char *strcherror (void);
char *strcherror_r (struct cherror_s *cherror);
But hey, if you're implementing it, your opinion counts
for a lot :)
:)
Derek
--
Derek R. Price
CVS Solutions Architect
Ximbiot http://ximbiot.com
v: +1 717.579.6168
f: +1 717.234.3125
mailto:[EMAIL PROTECTED]
___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price wrote: Hrm. Why isn't canon-host dependant on getaddrinfo? It would The alternative is that the ch_strerror_r function I've been working on would need to handle ENOMEM too, which introduces a dependency on strerror_r... I almost have the previously discussed canon-host code done but I would need to add a strerror_r module. Also, there is already duplicated code in canon-host getaddrinfo, which would be avoided by adding the ai_canonname functionality to getaddrinfo and dropping canon-host, or leaving it as a very thin wrapper. Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price wrote: This was about what I was thinking, though I was going to combine canon_host canon_host_r in the canon-host module and let the caller decide what to call. How about this API: Hrm. Why isn't canon-host dependant on getaddrinfo? It would simplify the canon-host code so much that it would probably be all right to LGPL it. It would also mean that canon-host could just return error codes suitable for gai_strerror. I'll have to extend lib/getaddrinfo.c a little to fill in ai_canonhost and add the gai_strerror function. Would that be acceptable? Come to think of it, after that, canon-host would be a pretty thin wrapper around getaddrinfo. Perhaps it would be best to tweak getaddrinfo and drop the canon-host module entirely? Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Derek Price [EMAIL PROTECTED] wrote: Derek Price wrote: Hrm. Why isn't canon-host dependant on getaddrinfo? It would The alternative is that the ch_strerror_r function I've been working on would need to handle ENOMEM too, which introduces a dependency on strerror_r... I almost have the previously discussed canon-host code done but I would need to add a strerror_r module. Also, there is already duplicated code in canon-host getaddrinfo, which would be avoided by adding the ai_canonname functionality to getaddrinfo and dropping canon-host, or leaving it as a very thin wrapper. Leaving canon_host as a thin wrapper sounds good. ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
Re: canon-host errors
Hrm. The POSIX getaddrinfo specification is pretty clear about *not* resolving a name for ai_canonname if name is in IP dot notation, yet sometimes the canon-host implemetation does a reverse-lookup to get a canonical name (for some odd condition where gethostbyname fills in the h_name field of the hostent struct with an IP address in dot notation). Does anyone actually know a system or set of circumstances where gethostbyname might actually return an IP number in dot notation when it is not given an IP number to resolve? There is some old code in CVS that implies that this should not happen, but the code path may be seldom-used since it has to do with Kerberos authentication. Regards, Derek -- Derek R. Price CVS Solutions Architect Ximbiot http://ximbiot.com v: +1 717.579.6168 f: +1 717.234.3125 mailto:[EMAIL PROTECTED] ___ bug-gnulib mailing list bug-gnulib@gnu.org http://lists.gnu.org/mailman/listinfo/bug-gnulib
