From 2c8f1efc80053f55afcd57930b43bca494883ae4 Mon Sep 17 00:00:00 2001 From: ahgamut <41098605+ahgamut@users.noreply.github.com> Date: Fri, 9 Jul 2021 16:37:03 +0530 Subject: [PATCH] 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 | 21 +++-- libc/dns/getservbyport.c | 21 +++-- libc/dns/servicestxt.c | 44 ++++++----- libc/dns/servicestxt.h | 8 +- test/libc/dns/servicestxt_test.c | 130 ++++++++++++++++++------------- 5 files changed, 121 insertions(+), 103 deletions(-) diff --git a/libc/dns/getservbyname.c b/libc/dns/getservbyname.c index c94711dc6..62fcf0168 100644 --- a/libc/dns/getservbyname.c +++ b/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; } diff --git a/libc/dns/getservbyport.c b/libc/dns/getservbyport.c index 8fd6359c5..fc4e21c49 100644 --- a/libc/dns/getservbyport.c +++ b/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; } diff --git a/libc/dns/servicestxt.c b/libc/dns/servicestxt.c index 238e4225f..cb45535d0 100644 --- a/libc/dns/servicestxt.c +++ b/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); } } } diff --git a/libc/dns/servicestxt.h b/libc/dns/servicestxt.h index 3a718fbb0..28e69eb37 100644 --- a/libc/dns/servicestxt.h +++ b/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? */ diff --git a/test/libc/dns/servicestxt_test.c b/test/libc/dns/servicestxt_test.c index 6ed97eb26..1f5b50bc8 100644 --- a/test/libc/dns/servicestxt_test.c +++ b/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"); }