Hi

sorry I have mixed up my patch files, I have attached the correct ones.

Philipp

[2024-01-25 13:16] Philipp Takacs <phil...@bureaucracy.de>
> [2024-01-25 10:15] Omar Polo <o...@omarpolo.com>
> > On 2024/01/24 08:51:06 +0100, Philipp <phil...@bureaucracy.de> wrote:
> > > [2024-01-24 00:09] Omar Polo <o...@omarpolo.com>
> > > > [...]
> > > > if you're interested in this however, we can also avoid the strdup()
> > > > here since aldap_parse_url() already strdup()s the string for parsing
> > > > (but still frees the passed argument...)
> > > 
> > > I have written two patches for this, one adding the free and one to
> > > avoid the unnecessary strdup.
> > > 
> > > Ass you might guess from the filenames, there are a few more patches. I'll
> > > send the rest after I have tested all my patches.
> >
> > They all read fine to me.  only one nitpick in the first one
> >
> > > From fa4cdb0a74c3b5d17cdc93b6285d765fda084740 Mon Sep 17 00:00:00 2001
> > > From: Philipp Takacs <phil...@bureaucracy.de>
> > > Date: Wed, 24 Jan 2024 01:16:56 +0100
> > > Subject: [PATCH 11/11] table-ldap aldap_parse_url now saves the port as 
> > > string
> > > [...]
> > > --- a/extras/tables/table-ldap/table_ldap.c
> > > +++ b/extras/tables/table-ldap/table_ldap.c
> > > @@ -118,7 +118,6 @@ ldap_connect(const char *addr)
> > >  {
> > >   struct aldap_url lu;
> > >   struct addrinfo  hints, *res0, *res;
> > > - char             port[32];
> > >   int              error, r, fd = -1;
> >
> > Here you could remove `r' too since it becomes unused.
>
> Thanks fixed.
>
> > If you've tested, I could commit my getaddrinfo() diff, then if you
> > rebase these one on top of it I could merge them.  (but it's also fine to
> > get the ldaps one in first, depending on how you prefer.)
>
> I have tested now your getaddrinfo rewrite and my two fixups.
> I have had some changes because they had been somewere in
> my rebase branche. clean versions of them are attached.
>
> Philipp
From d4fa732b5375e0fa8eaa0727ae82362c941bf8c1 Mon Sep 17 00:00:00 2001
From: Philipp Takacs <phil...@bureaucracy.de>
Date: Tue, 23 Jan 2024 13:58:47 +0100
Subject: [PATCH 2/3] table-ldap free aldap_url

---
 extras/tables/table-ldap/aldap.c      | 1 -
 extras/tables/table-ldap/table_ldap.c | 5 ++++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/extras/tables/table-ldap/aldap.c b/extras/tables/table-ldap/aldap.c
index d54a90c..552ea52 100644
--- a/extras/tables/table-ldap/aldap.c
+++ b/extras/tables/table-ldap/aldap.c
@@ -560,7 +560,6 @@ void
 aldap_free_url(struct aldap_url *lu)
 {
 	free(lu->buffer);
-	free(lu->filter);
 }
 
 int
diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c
index 6bdce67..79a26a8 100644
--- a/extras/tables/table-ldap/table_ldap.c
+++ b/extras/tables/table-ldap/table_ldap.c
@@ -122,13 +122,16 @@ ldap_connect(const char *addr)
 		if (fd == -1)
 			continue;
 
-		if (connect(fd, res->ai_addr, res->ai_addrlen) == 0)
+		if (connect(fd, res->ai_addr, res->ai_addrlen) == 0) {
+			aldap_free_url(&lu);
 			return aldap_init(fd);
+		}
 
 		close(fd);
 		fd = -1;
 	}
 
+	aldap_free_url(&lu);
 	return NULL;
 }
 
-- 
2.39.2

From dafbf8547e57b0f7142c8faf6ef776933502d151 Mon Sep 17 00:00:00 2001
From: Philipp Takacs <phil...@bureaucracy.de>
Date: Thu, 25 Jan 2024 12:47:46 +0100
Subject: [PATCH 3/3] table-ldap aldap_parse_url now saves the port as string

---
 extras/tables/table-ldap/aldap.c      |  3 ++-
 extras/tables/table-ldap/aldap.h      |  4 ++--
 extras/tables/table-ldap/table_ldap.c | 14 ++++----------
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/extras/tables/table-ldap/aldap.c b/extras/tables/table-ldap/aldap.c
index 552ea52..011a820 100644
--- a/extras/tables/table-ldap/aldap.c
+++ b/extras/tables/table-ldap/aldap.c
@@ -588,9 +588,10 @@ aldap_parse_url(char *url, struct aldap_url *lu)
 		/* if a port is given */
 		if (*(forward2+1) != '\0') {
 #define PORT_MAX UINT16_MAX
-			lu->port = strtonum(++forward2, 0, PORT_MAX, &errstr);
+			strtonum(++forward2, 0, PORT_MAX, &errstr);
 			if (errstr)
 				goto fail;
+			lu->port = forward2;
 		}
 	} else {
 		lu->port = LDAP_PORT;
diff --git a/extras/tables/table-ldap/aldap.h b/extras/tables/table-ldap/aldap.h
index 7cfd637..7217634 100644
--- a/extras/tables/table-ldap/aldap.h
+++ b/extras/tables/table-ldap/aldap.h
@@ -19,7 +19,7 @@
 #include "ber.h"
 
 #define LDAP_URL "ldap://";
-#define LDAP_PORT 389
+#define LDAP_PORT "389"
 #define LDAP_PAGED_OID  "1.2.840.113556.1.4.319"
 
 struct aldap {
@@ -71,7 +71,7 @@ enum aldap_protocol {
 struct aldap_url {
 	int		 protocol;
 	char		*host;
-	in_port_t	 port;
+	char		*port;
 	char		*dn;
 #define MAXATTR 1024
 	char		*attributes[MAXATTR];
diff --git a/extras/tables/table-ldap/table_ldap.c b/extras/tables/table-ldap/table_ldap.c
index 79a26a8..0f25c60 100644
--- a/extras/tables/table-ldap/table_ldap.c
+++ b/extras/tables/table-ldap/table_ldap.c
@@ -85,8 +85,8 @@ ldap_connect(const char *addr)
 {
 	struct aldap_url lu;
 	struct addrinfo	 hints, *res0, *res;
-	char		*buf, port[32];
-	int		 error, r, fd = -1;
+	char		*buf;
+	int		 error, fd = -1;
 
 	if ((buf = strdup(addr)) == NULL)
 		return NULL;
@@ -98,22 +98,16 @@ ldap_connect(const char *addr)
 		return NULL;
 	}
 
-	r = snprintf(port, sizeof(port), "%d", lu.port);
-	if (r < 0 || (size_t)r >= sizeof(port)) {
-		log_warnx("snprintf");
-		return NULL;
-	}
-
 	memset(&hints, 0, sizeof(hints));
 	hints.ai_family = PF_UNSPEC;
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_flags = AI_NUMERICSERV;
-	error = getaddrinfo(lu.host, port, &hints, &res0);
+	error = getaddrinfo(lu.host, lu.port, &hints, &res0);
 	if (error == EAI_AGAIN || error == EAI_NODATA || error == EAI_NONAME)
 		return NULL;
 	if (error) {
 		log_warnx("warn: could not parse \"%s:%s\": %s", lu.host,
-		    port, gai_strerror(error));
+		    lu.port, gai_strerror(error));
 		return NULL;
 	}
 
-- 
2.39.2

Reply via email to