söndag den 19 september 2010 klockan 06:02 skrev Alfred M. Szmidt detta:
>    +  * libinetutils/tftpsubs.c (synchnet): Upgrade the variable "from"
>    +  to be "struct sockaddr_storage".
> 
> Linguistically, you changed the type of FROM to `struct
> sockaddr_storage'; local variables should be in capital letters.  Thus
> this should be something like:
> 
> (synchnet): Changed type of FROM to `struct sockaddr_storage'.

I have reread the GNU standards document, but I could not locate any
explanations on the means available to differentiate between various
kinds of variables: global, local, statically scoped and so on.
Please quote me a reference so that I can learn this aspect properly,
and use it for all future contributions.

A new effort at the changelog has been attempted, but I will not
comment further on that. Any further comments are welcome, though.

> switch (from.ss_family)
>   {
>   case AF_INET: family = "IPv4"; break;
>   case AF_INET6: family = "IPv6"; break;
>   default: family = "?"; break;
>   }

Check! Slight variation incorprated.

>    +  static char host[NI_MAXHOST];
> 
> Does the Hurd have NI_MAXHOST? I don't know.

Made into a conditional clause just to be safe.

A new version of the patching is attached to this email.


Let me for free provide a strong reason to reject also the new patch,
a patch with which I myself am content:

   There is no trace of conditioning the changes on either of

       IPV6
       HAVE_IPV6
       HAVE_DECL_GETADDRINFO
       HAVE_DECL_FREEGETADDRINFO
       HAVE_DECL_GAI_STRERROR
       HAVE_DECL_GETNAMEINFO
       HAVE_STRUCT_SOCKADDR_STORAGE
       
   so my suggestion fails to compile on any machine where IPv6
   is not supported!

The particular files, "libinetutils/tftpsubs.c" and "src/tftpd.c", are
manageable in size to rework to recover from this calamity, but the other
contribution of mine on "src/tftp.c" is already so much more involved
that it is clearly less stimulating to rework the patching. Not to
mention "ftp/*" and "ftpd/*", my next goals. Making them fit for
systems with IPv6, and those with only IPv4, that is some serious major
work. Believe me, I have already patched linux-ftpd/netkit-ftpd and
netkit-ftp for Debian GNU/Linux, where I was allowed to assume the
presence of IPv6 support.

Could I get to hear some wise words on principles here? May IPv6 code
be implemented and committed first, and then afterwards be refined to
reimplement the fixes to take care of IPv4-only systems, or must both
settings be coped with in one single go? The answer will seriously
affect the labour and the pace with which the needed changes can be
implemented.


Regards,
Mats E A
From 696669406987bcac30aa66c232b7747e69f3d181 Mon Sep 17 00:00:00 2001
From: Mats Erik Andersson <g...@gisladisker.se>
Date: Fri, 24 Sep 2010 11:57:42 +0200
Subject: [PATCH] Implement IPv6 capability for the TFTP server.

---
 ChangeLog               |   14 ++++++++++
 libinetutils/tftpsubs.c |    2 +-
 src/tftpd.c             |   61 +++++++++++++++++++++++++++++++++-------------
 3 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1ff6282..1465b13 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2010-09-24  Mats Erik Andersson <g...@gisladisker.se>
+
+	* libinetutils/tftpsubs.c (synchnet): Changed type of FROM
+	to be "struct sockaddr_storage".
+
+	* src/tftpd.c: Changed all uses of "struct sockaddr_in"
+	to be "struct sockaddr_storage".
+	(fromlen): Meticulously tracks length of address structure FROM
+	in every function.
+	(verifyhost): Expanded declaration signature to be
+	"(struct sockaddr_storage *, socklen_t)".  Use getnameinfo(3)
+	as resolver.
+	(tftp) <logging>: Now displays address family "IPv4", "IPv6", or "?".
+
 2010-09-15  Mats Erik Andersson <g...@gisladisker.se>
 
 	* telnetd/telnetd.c (login_invocation, argp_options):
diff --git a/libinetutils/tftpsubs.c b/libinetutils/tftpsubs.c
index bc1f84a..6fa78bc 100644
--- a/libinetutils/tftpsubs.c
+++ b/libinetutils/tftpsubs.c
@@ -285,7 +285,7 @@ synchnet (int f)
 {
   int i, j = 0;
   char rbuf[PKTSIZE];
-  struct sockaddr_in from;
+  struct sockaddr_storage from;
   socklen_t fromlen;
 
   while (1)
diff --git a/src/tftpd.c b/src/tftpd.c
index 70602fa..b8bbe8d 100644
--- a/src/tftpd.c
+++ b/src/tftpd.c
@@ -101,7 +101,7 @@ static int maxtimeout = 5 * TIMEOUT;
 #define PKTSIZE	SEGSIZE+4
 static char buf[PKTSIZE];
 static char ackbuf[PKTSIZE];
-static struct sockaddr_in from;
+static struct sockaddr_storage from;
 static socklen_t fromlen;
 
 void tftp (struct tftphdr *, int);
@@ -124,7 +124,7 @@ static int logging;
 
 static const char *errtomsg (int);
 static void nak (int);
-static const char *verifyhost (struct sockaddr_in *);
+static const char *verifyhost (struct sockaddr_storage *, socklen_t);
 
 
 
@@ -172,7 +172,7 @@ main (int argc, char *argv[])
   int index;
   register struct tftphdr *tp;
   int on, n;
-  struct sockaddr_in sin;
+  struct sockaddr_storage sin;
 
   set_program_name (argv[0]);
   iu_argp_init ("tftpd", default_program_authors);
@@ -268,24 +268,29 @@ main (int argc, char *argv[])
 	exit (EXIT_SUCCESS);
       }
   }
-  from.sin_family = AF_INET;
+
   alarm (0);
   close (0);
   close (1);
-  peer = socket (AF_INET, SOCK_DGRAM, 0);
+
+  /* The peer's address 'from' is valid at this point.
+   * 'from.ss_family' contains the correct address
+   * family for any callback connection, and 'fromlen'
+   * is the length of the corresponding address structure.  */
+  peer = socket (from.ss_family, SOCK_DGRAM, 0);
   if (peer < 0)
     {
       syslog (LOG_ERR, "socket: %m\n");
       exit (EXIT_FAILURE);
     }
   memset (&sin, 0, sizeof (sin));
-  sin.sin_family = AF_INET;
-  if (bind (peer, (struct sockaddr *) &sin, sizeof (sin)) < 0)
+  sin.ss_family = from.ss_family;
+  if (bind (peer, (struct sockaddr *) &sin, fromlen) < 0)
     {
       syslog (LOG_ERR, "bind: %m\n");
       exit (EXIT_FAILURE);
     }
-  if (connect (peer, (struct sockaddr *) &from, sizeof (from)) < 0)
+  if (connect (peer, (struct sockaddr *) &from, fromlen) < 0)
     {
       syslog (LOG_ERR, "connect: %m\n");
       exit (EXIT_FAILURE);
@@ -360,8 +365,22 @@ again:
   ecode = (*pf->f_validate) (&filename, tp->th_opcode);
   if (logging)
     {
-      syslog (LOG_INFO, "%s: %s request for %s: %s",
-	      verifyhost (&from),
+      char *family;
+
+      switch (from.ss_family)
+	{
+	case AF_INET:
+	  family = "IPv4";
+	  break;
+	case AF_INET6:
+	  /* Should mapped IPv4 addresses be reported?  */
+	  family = "IPv6";
+	  break;
+	default:
+	  family = "?";
+	}
+      syslog (LOG_INFO, "%s (%s): %s request for %s: %s",
+	      verifyhost (&from, fromlen), family,
 	      tp->th_opcode == WRQ ? "write" : "read",
 	      filename, errtomsg (ecode));
     }
@@ -737,16 +756,24 @@ nak (int error)
 }
 
 static const char *
-verifyhost (struct sockaddr_in *fromp)
+verifyhost (struct sockaddr_storage *fromp, socklen_t frlen)
 {
-  struct hostent *hp;
+  int rc;
+#ifdef NI_MAXHOST
+  static char host[NI_MAXHOST];
+#else
+  static char host[64];
+#endif
 
-  hp = gethostbyaddr ((char *) &fromp->sin_addr, sizeof (fromp->sin_addr),
-		      fromp->sin_family);
-  if (hp)
-    return hp->h_name;
+  rc = getnameinfo ((struct sockaddr *) fromp, frlen,
+		    host, sizeof (host), NULL, 0, 0);
+  if (rc == 0)
+    return host;
   else
-    return inet_ntoa (fromp->sin_addr);
+    {
+      syslog (LOG_ERR, "getnameinfo: %s\n", gai_strerror(rc));
+      return "0.0.0.0";
+    }
 }
 
 static const char usage_str[] =
-- 
1.7.0

Attachment: signature.asc
Description: Digital signature

Reply via email to