Hi Erik,
Erik Auerswald <auers...@unix-ag.uni-kl.de> writes:
>> Thanks for looking into fixing warnings! :-)
There are many of them, so feel free to work on them. I won't be angry
if you finish fixing them before me. :)
>> > - snprintf (portstr, sizeof (portstr) - 1, "%u", port);
>> > + sprintf (portstr, "%u", port);
>> > memset (&hisctladdr, 0, sizeof (hisctladdr));
>> > memset (&hints, 0, sizeof (hints));
>> >
>>
>> I prefer the original code with snprintf() here, because it directly
>> shows that it does not overflow the buffer and that the last byte of
>> the buffer is unchanged.
>
> I'd still prefer keeping snprintf() here.
I was going to disagree since that macro allows all values between 0 -
65535.
However, I saw an old thread on bug-gnu...@gnu.org [1]. Apparently,
POSIX requires '%d' to use the portable character set (which grantees 1
byte per character). Other specifiers can do as they wish.
In practice, I don't think it should ever be a problem. But I guess
snprintf would be safer in that case...
Applied the patch changing the sprintf to:
+ snprintf (portstr, sizeof portstr, "%u", port);
Patch attached. Feel free to make changes if you see fit.
Collin
[1] https://lists.gnu.org/archive/html/bug-gnulib/2011-02/msg00119.html
>From e29e76b37b31eef27c5d53cc87306e7b80376762 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Wed, 31 Jul 2024 22:14:01 -0700
Subject: [PATCH] ftp: Cleanup port number to string conversion.
* bootstrap.conf (gnulib_modules): Add intprops.
* ftp/ftp.c: Include intprops.h.
(portstr): Size buffer to fit exactly 'in_port_t' and a NUL byte.
(hookup): Use 'in_port_t' to represent the port instead of int. Use
sprintf since the buffer size is safe.
* ftp/extern.h (hookup): Adjust deceleration.
* ftp/ftp_var.h: Include netinet/in.h for type definitions.
---
bootstrap.conf | 1 +
ftp/extern.h | 2 +-
ftp/ftp.c | 8 +++++---
ftp/ftp_var.h | 3 +++
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 9026eccc..e6f56a27 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -68,6 +68,7 @@ getusershell
git-version-gen
gitlog-to-changelog
glob
+intprops
inttostr
ioctl
malloc-gnu
diff --git a/ftp/extern.h b/ftp/extern.h
index f773ec82..e339ed72 100644
--- a/ftp/extern.h
+++ b/ftp/extern.h
@@ -80,7 +80,7 @@ int getreply (int);
char *globulize (char *);
char *gunique (char *);
void help (int, char **);
-char *hookup (char *, int);
+char *hookup (char *, in_port_t);
void site_idle (int, char **);
int initconn (void);
void intr (int sig);
diff --git a/ftp/ftp.c b/ftp/ftp.c
index 9ec40a70..60bd2688 100644
--- a/ftp/ftp.c
+++ b/ftp/ftp.c
@@ -88,6 +88,7 @@
# include <idna.h>
#endif
+#include <intprops.h>
#include <timespec.h>
#include "ftp_var.h"
@@ -107,7 +108,7 @@ socklen_t ctladdrlen; /* Applies to all addresses. */
/* For temporary resolving: hookup() and initconn()/noport. */
static char ia[INET6_ADDRSTRLEN];
-static char portstr[10];
+static char portstr[INT_STRLEN_BOUND (in_port_t) + 1];
FILE *cin, *cout;
@@ -116,7 +117,7 @@ FILE *cin, *cout;
#endif
char *
-hookup (char *host, int port)
+hookup (char *host, in_port_t port)
{
struct addrinfo hints, *ai = NULL, *res = NULL;
struct timeval timeout;
@@ -142,7 +143,8 @@ hookup (char *host, int port)
rhost = strdup (host);
#endif
- snprintf (portstr, sizeof (portstr) - 1, "%u", port);
+
+ snprintf (portstr, sizeof portstr, "%u", port);
memset (&hisctladdr, 0, sizeof (hisctladdr));
memset (&hints, 0, sizeof (hints));
diff --git a/ftp/ftp_var.h b/ftp/ftp_var.h
index 63dc8d82..f1e7746c 100644
--- a/ftp/ftp_var.h
+++ b/ftp/ftp_var.h
@@ -53,6 +53,9 @@
#include <setjmp.h>
#include <progname.h>
+/* For in_port_t. */
+#include <netinet/in.h>
+
#ifndef FTP_EXTERN
# define FTP_EXTERN extern
#endif
--
2.45.2
