avoid malloc/free wrt LookupServicesByX

the getservbyX functions allow to specify the protocol as NULL, in which
case services with either protocol can be matched. The previous solution
this involved sending a char **localproto to LookupServicesByX,
allocating *localproto at the callee if it was NULL, and then have the
caller free it if the allocation did take place.

This gets ugly on the caller side, especially with error handling added
to the mix: for example, getnameinfo requires a LookupServicesByPort
call, which requires error-checking, null-checking, and freeing what was
basically a temporary string anyway.

Now the LookupServicesByX functions expect a non-NULL buffer + non-zero
length for localproto, and will write the protocol to the buffer if it
is an empty string. This makes it simpler to handle from the caller
side (can now pass constant strings as well).

Also, you can pass NULL for the official name in LookupServicesByName,
since some callers will want only the port value.

libc/dns/getservbyname.c

@@ -28,33 +28,30 @@
 #include "libc/dns/ent.h"
 #include "libc/dns/servicestxt.h"
 #include "libc/mem/mem.h"
+#include "libc/str/str.h"
 
 struct servent *getservbyname(const char *name, const char *proto) {
   static struct servent *ptr0, se0;
   static char s_name[DNS_NAME_MAX + 1];
-  char *localproto = proto;
+  static char localproto[DNS_NAME_MAX + 1];
   int p;
 
   if (!ptr0) {
     se0.s_name = s_name;
     if (!(se0.s_aliases = calloc(1, sizeof(char *)))) return NULL;
     se0.s_port = 0;
-    se0.s_proto = NULL;
+    se0.s_proto = localproto;
     ptr0 = &se0;
   }
 
-  p = LookupServicesByName(name, &localproto, ptr0->s_name, DNS_NAME_MAX, NULL);
-  if (p == -1) {
-    // localproto got alloc'd during the lookup?
-    if (!proto && localproto != proto) free(localproto);
-    return NULL;
-  }
+  localproto[0] = '\0';
+  if (proto) strncpy(localproto, proto, DNS_NAME_MAX);
+
+  p = LookupServicesByName(name, ptr0->s_proto, DNS_NAME_MAX, ptr0->s_name,
+                           DNS_NAME_MAX, NULL);
+  if (p == -1) return NULL;
 
   ptr0->s_port = htons(p);
-  if (ptr0->s_proto) free(ptr0->s_proto);
-  ptr0->s_proto = strdup(localproto);
-
-  if (!proto && localproto != proto) free(localproto);
 
   return ptr0;
 }

libc/dns/getservbyport.c

@@ -27,32 +27,29 @@
 ╚─────────────────────────────────────────────────────────────────────────────*/
 #include "libc/dns/ent.h"
 #include "libc/dns/servicestxt.h"
+#include "libc/mem/mem.h"
+#include "libc/str/str.h"
 
 struct servent *getservbyport(int port, const char *proto) {
   static struct servent *ptr1, se1;
   static char s_name[DNS_NAME_MAX + 1];
-  char *localproto = proto;
+  static char localproto[DNS_NAME_MAX + 1];
 
   if (!ptr1) {
     se1.s_name = s_name;
     if (!(se1.s_aliases = calloc(1, sizeof(char *)))) return NULL;
     se1.s_port = 0;
-    se1.s_proto = NULL;
+    se1.s_proto = localproto;
     ptr1 = &se1;
   }
 
-  if (LookupServicesByPort(ntohs(port), &localproto, ptr1->s_name, DNS_NAME_MAX,
-                           NULL) == -1) {
-    // localproto got alloc'd during the lookup?
-    if (!proto && localproto != proto) free(localproto);
+  localproto[0] = '\0';
+  if (proto) strncpy(localproto, proto, DNS_NAME_MAX);
+
+  if (LookupServicesByPort(ntohs(port), ptr1->s_proto, DNS_NAME_MAX,
+                           ptr1->s_name, DNS_NAME_MAX, NULL) == -1)
     return NULL;
-  }
 
   ptr1->s_port = port;
-  if (ptr1->s_proto) free(ptr1->s_proto);
-  ptr1->s_proto = strdup(localproto);
-
-  if (!proto && localproto != proto) free(localproto);
-
   return ptr1;
 }

libc/dns/servicestxt.c

@@ -64,7 +64,11 @@ static textwindows noinline char *GetNtServicesTxtPath(char *pathbuf,
  *      ssh		    22/tcp
  *
  * @param servport is the port number
- * @param servproto is a pointer to a string (*servproto can be NULL)
+ * @param servproto is a NULL-terminated string (eg "tcp", "udp")
+ *          (if servproto is an empty string,
+ *           if is filled with the first matching
+ *           protocol)
+ * @param servprotolen the size of servproto
  * @param buf is a buffer to store the official name of the service
  * @param bufsize is the size of buf
  * @param filepath is the location of the services file
@@ -73,8 +77,9 @@ static textwindows noinline char *GetNtServicesTxtPath(char *pathbuf,
  *
  * @note aliases are not read from the file.
  */
-int LookupServicesByPort(const int servport, char **servproto, char *buf,
-                         size_t bufsize, const char *filepath) {
+int LookupServicesByPort(const int servport, char *servproto,
+                         size_t servprotolen, char *buf, size_t bufsize,
+                         const char *filepath) {
   FILE *f;
   char *line;
   char pathbuf[PATH_MAX];
@@ -91,7 +96,7 @@ int LookupServicesByPort(const int servport, char **servproto, char *buf,
     }
   }
 
-  if (bufsize == 0 || !(f = fopen(path, "r"))) {
+  if (servprotolen == 0 || bufsize == 0 || !(f = fopen(path, "r"))) {
     return -1;
   }
   line = NULL;
@@ -104,13 +109,10 @@ int LookupServicesByPort(const int servport, char **servproto, char *buf,
     port = strtok_r(NULL, "/ \t\r\n\v", &tok);
     proto = strtok_r(NULL, " \t\r\n\v", &tok);
     if (name && port && proto && servport == atoi(port)) {
-      if (!servproto[0]) {
-        servproto[0] = strdup(proto);
-        strncpy(buf, name, bufsize);
-        found = 1;
-      } else if (strcasecmp(proto, servproto[0]) == 0) {
+      if (!servproto[0] || strncasecmp(proto, servproto, servprotolen) == 0) {
         strncpy(buf, name, bufsize);
         found = 1;
+        if (!servproto[0]) strncpy(servproto, proto, servprotolen);
       }
     }
   }
@@ -131,8 +133,13 @@ int LookupServicesByPort(const int servport, char **servproto, char *buf,
  * Opens and searches /etc/services to find port for a given name.
  *
  * @param servname is a NULL-terminated string
- * @param servproto is a pointer to a string (*servproto can be NULL)
+ * @param servproto is a NULL-terminated string (eg "tcp", "udp")
+ *          (if servproto is an empty string,
+ *           if is filled with the first matching
+ *           protocol)
+ * @param servprotolen the size of servproto
  * @param buf is a buffer to store the official name of the service
+ *          (if NULL, the official name is not stored)
  * @param bufsize is the size of buf
  * @param filepath is the location of services file
  *          (if NULL, uses /etc/services)
@@ -142,8 +149,9 @@ int LookupServicesByPort(const int servport, char **servproto, char *buf,
  * @note aliases are read from file for comparison, but not returned.
  * @see LookupServicesByPort
  */
-int LookupServicesByName(const char *servname, char **servproto, char *buf,
-                         size_t bufsize, const char *filepath) {
+int LookupServicesByName(const char *servname, char *servproto,
+                         size_t servprotolen, char *buf, size_t bufsize,
+                         const char *filepath) {
   FILE *f;
   char *line;
   char pathbuf[PATH_MAX];
@@ -160,7 +168,7 @@ int LookupServicesByName(const char *servname, char **servproto, char *buf,
     }
   }
 
-  if (bufsize == 0 || !(f = fopen(path, "r"))) {
+  if (servprotolen == 0 || !(f = fopen(path, "r"))) {
     return -1;
   }
   line = NULL;
@@ -180,15 +188,11 @@ int LookupServicesByName(const char *servname, char **servproto, char *buf,
 
       if (alias) /* alias matched with servname */
       {
-        if (!servproto[0]) {
-          servproto[0] = strdup(proto);
+        if (!servproto[0] || strncasecmp(proto, servproto, servprotolen) == 0) {
           result = atoi(port);
-          strncpy(buf, name, bufsize);
-          found = 1;
-        } else if (strcasecmp(proto, servproto[0]) == 0) {
-          result = atoi(port);
-          strncpy(buf, name, bufsize);
           found = 1;
+          if (!servproto[0]) strncpy(servproto, proto, servprotolen);
+          if (buf && bufsize != 0) strncpy(buf, name, bufsize);
         }
       }
     }

libc/dns/servicestxt.h

@@ -6,10 +6,10 @@
 #if !(__ASSEMBLER__ + __LINKER__ + 0)
 COSMOPOLITAN_C_START_
 
-int LookupServicesByPort(const int, char **, char *, size_t, const char *)
-    paramsnonnull((2, 3));
-int LookupServicesByName(const char *, char **, char *, size_t, const char *)
-    paramsnonnull((1, 2, 3));
+int LookupServicesByPort(const int, char *, size_t, char *, size_t,
+                         const char *) paramsnonnull((2, 4));
+int LookupServicesByName(const char *, char *, size_t, char *, size_t,
+                         const char *) paramsnonnull((1, 2));
 
 /* TODO: implement like struct HostsTxt? */
 

test/libc/dns/servicestxt_test.c

@@ -48,87 +48,107 @@ ssh		    22/tcp		# SSH Remote Login Protocol";
 }
 
 TEST(LookupServicesByPort, GetNameWhenPortCorrect) {
-  char name[16]; /* sample has only names of length 3 */
+  char name[8];        /* service names are of length 3 */
+  char eitherproto[8]; /* protocol names are of length 3 */
   char proto1[] = "tcp";
   char proto2[] = "udp";
-  char* localproto[1];
+  char* localproto;
 
-  localproto[0] = NULL;
-  ASSERT_EQ(
-      -1, /* non existent port */
-      LookupServicesByPort(965, localproto, name, sizeof(name), "services"));
-  ASSERT_EQ(NULL, localproto[0]);
+  eitherproto[0] = '\0';
 
+  localproto = eitherproto;
+  ASSERT_EQ(-1, /* non existent port */
+            LookupServicesByPort(965, localproto, sizeof(eitherproto), name,
+                                 sizeof(name), "services"));
+  ASSERT_EQ('\0', localproto[0]);
+
+  localproto = eitherproto;
   ASSERT_EQ(-1, /* port in network byte order */
-            LookupServicesByPort(htons(22), localproto, name, sizeof(name),
-                                 "services"));
-  ASSERT_EQ(NULL, localproto[0]);
+            LookupServicesByPort(htons(22), localproto, sizeof(eitherproto),
+                                 name, sizeof(name), "services"));
+  ASSERT_EQ('\0', localproto[0]);
 
-  localproto[0] = proto2;
-  ASSERT_EQ(
-      -1, /* port ok but wrong protocol */
-      LookupServicesByPort(22, localproto, name, sizeof(name), "services"));
-  ASSERT_EQ(localproto[0], proto2);
+  localproto = proto2;
+  ASSERT_EQ(-1, /* port ok but wrong protocol */
+            LookupServicesByPort(22, localproto, sizeof(proto2), name,
+                                 sizeof(name), "services"));
+  ASSERT_STREQ(proto2, "udp");
 
-  localproto[0] = proto1;
+  localproto = proto1;
   ASSERT_EQ(
-      0, LookupServicesByPort(22, localproto, name, sizeof(name), "services"));
+      -1, /* protocol is non-NULL/length must be nonzero */
+      LookupServicesByPort(22, localproto, 0, name, sizeof(name), "services"));
+  ASSERT_STREQ(proto1, "tcp");
+
+  localproto = proto1;
+  ASSERT_EQ(0, LookupServicesByPort(22, localproto, sizeof(proto1), name,
+                                    sizeof(name), "services"));
   ASSERT_STREQ(name, "ssh");
-  ASSERT_EQ(localproto[0], proto1);
+  ASSERT_STREQ(proto1, "tcp");
 
-  localproto[0] = proto2;
-  ASSERT_EQ(
-      0, LookupServicesByPort(19, localproto, name, sizeof(name), "services"));
+  localproto = proto2;
+  ASSERT_EQ(0, LookupServicesByPort(19, localproto, sizeof(proto2), name,
+                                    sizeof(name), "services"));
   ASSERT_STREQ(name, "chargen");
-  ASSERT_EQ(localproto[0], proto2);
+  ASSERT_STREQ(proto2, "udp");
 
-  localproto[0] = NULL;
-  ASSERT_EQ(
-      0, /* pick first matching protocol */
-      LookupServicesByPort(19, localproto, name, sizeof(name), "services"));
+  localproto = eitherproto;
+  ASSERT_EQ(0, /* pick first matching protocol */
+            LookupServicesByPort(19, localproto, sizeof(eitherproto), name,
+                                 sizeof(name), "services"));
   ASSERT_STREQ(name, "chargen");
-  ASSERT_NE(NULL, localproto[0]); /* got alloc'd during the call */
-  ASSERT_STREQ(localproto[0], "tcp");
-  free(localproto[0]);
+  ASSERT_NE('\0', localproto[0]); /* buffer filled during the call */
+  ASSERT_STREQ(eitherproto, "tcp");
 }
 
 TEST(LookupServicesByName, GetPortWhenNameOrAlias) {
-  char name[16]; /* sample has only names of length 3 */
+  char name[8];        /* service names are of length 3 */
+  char eitherproto[8]; /* protocol names are of length 3 */
   char proto1[] = "tcp";
   char proto2[] = "udp";
-  char* localproto[1];
+  char* localproto;
+  eitherproto[0] = '\0';
 
-  localproto[0] = NULL;
-  ASSERT_EQ(
-      -1, /* non-existent name */
-      LookupServicesByName("http", localproto, name, sizeof(name), "services"));
-  ASSERT_EQ(NULL, localproto[0]);
+  localproto = eitherproto;
+  ASSERT_EQ(-1, /* non-existent name */
+            LookupServicesByName("http", localproto, sizeof(eitherproto), name,
+                                 sizeof(name), "services"));
+  ASSERT_EQ('\0', localproto[0]);
 
-  localproto[0] = proto2;
-  ASSERT_EQ(
-      -1, /* name exists but wrong protocol */
-      LookupServicesByName("ssh", localproto, name, sizeof(name), "services"));
-  ASSERT_EQ(localproto[0], proto2);
+  localproto = proto2;
+  ASSERT_EQ(-1, /* name exists but wrong protocol */
+            LookupServicesByName("ssh", localproto, sizeof(proto2), name,
+                                 sizeof(name), "services"));
+  ASSERT_STREQ(proto2, "udp");
 
-  localproto[0] = proto1;
-  ASSERT_EQ(22, LookupServicesByName("ssh", localproto, name, sizeof(name),
-                                     "services"));
+  localproto = proto2;
+  ASSERT_EQ(-1, /* protocol is non-NULL/length must be nonzero */
+            LookupServicesByName("ssh", localproto, 0, name, sizeof(name),
+                                 "services"));
+  ASSERT_STREQ(proto2, "udp");
+
+  localproto = proto1;
+  ASSERT_EQ(22, LookupServicesByName("ssh", localproto, sizeof(proto1), name,
+                                     sizeof(name), "services"));
   ASSERT_STREQ(name, "ssh"); /* official name written to buffer */
-  ASSERT_EQ(localproto[0], proto1);
+  ASSERT_STREQ(proto1, "tcp");
 
-  localproto[0] = proto2;
+  localproto = proto2;
   ASSERT_EQ(19, /* works if alias provided */
-            LookupServicesByName("ttytst", localproto, name, sizeof(name),
-                                 "services"));
+            LookupServicesByName("ttytst", localproto, sizeof(proto2), name,
+                                 sizeof(name), "services"));
   ASSERT_STREQ(name, "chargen"); /* official name written to buffer */
-  ASSERT_EQ(localproto[0], proto2);
 
-  localproto[0] = NULL;
-  ASSERT_EQ(19, /* pick first matching protocol */
-            LookupServicesByName("source", localproto, name, sizeof(name),
+  localproto = proto2;
+  ASSERT_EQ(19, /* can get port returned without official name */
+            LookupServicesByName("ttytst", localproto, sizeof(proto2), NULL, 0,
                                  "services"));
+
+  name[0] = '\0';
+  localproto = eitherproto;
+  ASSERT_EQ(19, /* pick first matching protocol */
+            LookupServicesByName("source", localproto, sizeof(eitherproto),
+                                 name, sizeof(name), "services"));
   ASSERT_STREQ(name, "chargen");
-  ASSERT_NE(NULL, localproto[0]); /* got alloc'd during the call */
-  ASSERT_STREQ(localproto[0], "tcp");
-  free(localproto[0]);
+  ASSERT_STREQ(localproto, "tcp");
 }