This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix NIS getservbyproto plus another performance tweak for NULL protocol


Hi!

POSIX says getservbyproto's first argument is in network byte order.
Unfortunately _nss_nis_getservbyproto_r forget to ntohs it when snprintfing,
so it issued a request for a wrong service with yp_match on little endian
boxes.  If lucky, it would not be a valid service and sequential scanning
was done, if unlucky, wrong service was returned.

Another thing is a performance tweak for protocol == NULL.  "tcp" and "udp"
are the most often used protocols, so libnss_nis.so can first try
*/tcp, then */udp and then fallback to the current sequential scanning.
NIS doesn't comply to POSIX anyway in that it doesn't guarantee that even
during sequential YPPROC_ALL scanning of services.by{,service}name original
first service will be found first, so scanning for tcp and udp first doesn't
make things worse in this regard.

Plus there are 2 small tweaks, one is to use always services.byname map
for sequential scanning (the old code did that and the comment sounded
like there could be NIS servers without services.byservicename but with
services.byname) and the second one is not to allocate excessive 100 chars
for %d integer.

2004-03-30  Jakub Jelinek  <jakub@redhat.com>

	* nis/nss_nis/nis-service.c (_nss_nis_getservbyname_r): If protocol
	== NULL, try name/tcp and name/udp first before falling back into
	the sequential scanning.  Use services.byname database for
	sequential scanning.
	(_nss_nis_getservbyport_r): Likewise.  Just allocate sizeof (int) * 3
	chars for integer.

	* nis/nss_nis/nis-service.c (_nss_nis_getservbyport_r): Convert
	proto to host by order for snprintf.

--- libc/nis/nss_nis/nis-service.c.jj	2004-03-30 22:15:38.000000000 +0200
+++ libc/nis/nss_nis/nis-service.c	2004-03-30 23:03:03.899449997 +0200
@@ -268,6 +268,7 @@ _nss_nis_getservbyname_r (const char *na
 {
   enum nss_status status;
   char *domain;
+  const char *proto;
 
   if (name == NULL)
     {
@@ -279,18 +280,21 @@ _nss_nis_getservbyname_r (const char *na
     return NSS_STATUS_UNAVAIL;
 
   /* If the protocol is given, we could try if our NIS server knows
-     about services.byservicename map. If yes, we only need one query */
-  if (protocol != NULL)
+     about services.byservicename map. If yes, we only need one query.
+     If the protocol is not given, try first name/tcp, then name/udp
+     and then fallback to sequential scanning of services.byname map.  */
+  proto = protocol != NULL ? protocol : "tcp";
+  do
     {
-      char key[strlen (name) + strlen (protocol) + 2];
+      char key[strlen (name) + strlen (proto) + 2];
       char *cp, *result;
       size_t keylen, len;
       int int_len;
 
-      /* key is: "name/protocol" */
+      /* key is: "name/proto" */
       cp = stpcpy (key, name);
       *cp++ = '/';
-      stpcpy (cp, protocol);
+      stpcpy (cp, proto);
       keylen = strlen (key);
       status = yperr2nss (yp_match (domain, "services.byservicename", key,
 				    keylen, &result, &int_len));
@@ -329,6 +333,7 @@ _nss_nis_getservbyname_r (const char *na
 	    return NSS_STATUS_SUCCESS;
 	}
     }
+  while (protocol == NULL && (proto[0] == 't' ? (proto = "udp") : NULL));
 
   struct ypall_callback ypcb;
   struct search_t req;
@@ -343,7 +348,7 @@ _nss_nis_getservbyname_r (const char *na
   req.buflen = buflen;
   req.errnop = errnop;
   req.status = NSS_STATUS_NOTFOUND;
-  status = yperr2nss (yp_all (domain, "services.byservicename", &ypcb));
+  status = yperr2nss (yp_all (domain, "services.byname", &ypcb));
 
   if (status != NSS_STATUS_SUCCESS)
     return status;
@@ -358,20 +363,24 @@ _nss_nis_getservbyport_r (int port, cons
 {
   enum nss_status status;
   char *domain;
+  const char *proto;
 
   if (yp_get_default_domain (&domain))
     return NSS_STATUS_UNAVAIL;
 
-  /* If the protocol is given, we only need one query */
-  if (protocol != NULL)
+  /* If the protocol is given, we only need one query.
+     Otherwise try first port/tcp, then port/udp and then fallback
+     to sequential scanning of services.byname.  */
+  proto = protocol != NULL ? protocol : "tcp";
+  do
     {
-      char key[100 + strlen (protocol) + 2];
+      char key[sizeof (int) * 3 + strlen (proto) + 2];
       char *result;
       size_t keylen, len;
       int int_len;
 
-      /* key is: "port/protocol" */
-      keylen = snprintf (key, sizeof (key), "%d/%s", port, protocol);
+      /* key is: "port/proto" */
+      keylen = snprintf (key, sizeof (key), "%d/%s", ntohs (port), proto);
       status = yperr2nss (yp_match (domain, "services.byname", key,
 				    keylen, &result, &int_len));
       len = int_len;
@@ -409,6 +418,7 @@ _nss_nis_getservbyport_r (int port, cons
 	    return NSS_STATUS_SUCCESS;
 	}
     }
+  while (protocol == NULL && (proto[0] == 't' ? (proto = "udp") : NULL));
 
   if (port == -1)
     return NSS_STATUS_NOTFOUND;

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]