[Nut-upsdev] Default NUT PORT (errrr...)

[Peter Selinger]

Arjen de Korte wrote:
> 
> Arjen de Korte wrote:
> 
> >> I think a better solution is to just #define PORT as a character
> >> string in the first place. As far as I can see, this primarily affects
> >> two lines: include/config.h:197 (i.e. configure.in:505) and
> >> clients/upsclient.c:966.
> > That might be an option later on. But since that requires a change of the
> > upsclient interface, that was not my first thought.
> 
> In addition to the above, the preprocessor can convert a numeric
> *constant* to a string, but the other way around can only be done at
> runtime as far as I know. So unless we change the port in upsclient.c to
> a string instead of an int, this is counter productive.

I am not convinced this is the best solution. You said you wanted to
avoid runtime conversions, but for the 99% of users that use IPv4, you
just introduced such a conversion in upsd.c, line 204. The string
constant manufactured by the preprocessor is converted back to an
integer there. 

The use of getaddrinfo() is not in itself a good reason to make the
port a string type. That function is probably not portable anyway.

Perhaps one good reason for making the port a string is to allow
symbolic names, such as "http" instead of "80". I don't know how
useful this will be in NUT - would anybody want to run a NUT server on
a port reserved for some other protocol? 

In this case, --with-port should also take a string, and therefore
PORT should be #defined as a string. I don't think the interface in
upsclient.c needs to be changed at all; all that's needed is for
upscli_splitname() to call getservbyname(3) to generate a numeric
value.

How about defining a function to convert a string to a port number, as
follows:

#include <netdb.h>
#include <stdlib.h>
#include <netinet/in.h>

int strtoport(char *s) {
   int port;
   char *endptr;
   struct servent *service;

   port = strtol(s, &endptr, 10);
   if (endptr != s && *endptr == '\0') {
      return port;
   }
   service = getservbyname(s, NULL);  /* specify proto = tcp or not? */
   if (service) {
      return ntohs(service->s_port);
   }
   return 0;  /* invalid port */
}

This returns for example
strtoport("1234") == 1234
strtoport("http") == 80

Coupling this with the following patch I think is all that is needed
to enable symbolic port names throughout.  The only remaining problem
is where to put the function strtoport; it would have to be somewhere
both upsd and upsclient can use it.

What do you think? -- Peter

Index: configure.in
===================================================================
--- configure.in	(revision 733)
+++ configure.in	(working copy)
@@ -502,7 +502,7 @@
 	esac],
 	PORT="3493"
 )
-AC_DEFINE_UNQUOTED(PORT, ${PORT}, [Port for network communications])
+AC_DEFINE_UNQUOTED(PORT, "${PORT}", [Port for network communications])
 AC_MSG_RESULT(${PORT})
 
 AC_MSG_CHECKING(user to run as)
Index: clients/upsclient.c
===================================================================
--- clients/upsclient.c	(revision 733)
+++ clients/upsclient.c	(working copy)
@@ -962,11 +962,12 @@
 	/* skip the separator between hostname and port (if any) */
 	if (((s = strtok(NULL, "\0")) != NULL) && (*s == ':')) s++;
 
-	if (s == NULL)
-		*port = PORT;
-	else
-		*port = strtol(s, NULL, 10);
+	if (s == NULL) {
+		s = PORT;
+	}
 
+	*port = str_to_port(s);
+
 	return 0;
 }
 
Index: server/upsd.c
===================================================================
--- server/upsd.c	(revision 733)
+++ server/upsd.c	(working copy)
@@ -201,7 +201,7 @@
 
 	memset(&server, '\0', sizeof(server));
 	server.sin_family = AF_INET;
-	server.sin_port = htons(atoi(serv->port));
+	server.sin_port = htons(str_to_port(serv->port));
 
 	memcpy(&server.sin_addr, host->h_addr, host->h_length);
 
@@ -994,7 +994,7 @@
 
 	/* default behaviour if no LISTEN addres has been specified */
 	if (firstaddr == NULL)
-		listen_add("0.0.0.0", string_const(PORT));
+		listen_add("0.0.0.0", PORT);
 
 	for (serv = firstaddr; serv != NULL; serv = serv->next)
 		setuptcp(serv);
Index: server/conf.c
===================================================================
--- server/conf.c	(revision 733)
+++ server/conf.c	(working copy)
@@ -200,7 +200,7 @@
 	/* LISTEN <address> [<port>] */
 	if (!strcmp(arg[0], "LISTEN")) {
 		if (numargs < 3)
-			listen_add(arg[1], string_const(PORT));
+			listen_add(arg[1], PORT);
 		else
 			listen_add(arg[1], arg[2]);
 		return 1;