[ofa-general] [PATCH] Fix further bugs around console closure and clean up code.

[Ira Weiny]

From: Ira Weiny <weiny2 at llnl.gov>
Date: Tue, 7 Apr 2009 16:46:18 -0700
Subject: [PATCH] Fix further bugs around console closure and clean up code.

	It was found that closing the socket connection forcibly from a new
	connection would close the socket and leave the last connection
	unresponsive to "quit"

	The reproducer was:

	> ./opensm -B -console socket

	(Open connection #1)
	> telnet localhost 10000
	Trying 127.0.0.1...
	Connected to localhost.localdomain (127.0.0.1).
	Escape character is '^]'.
	OpenSM $

	(From another terminal open connection #2)
	> telnet localhost 10000
	Trying 127.0.0.1...
	Connected to localhost.localdomain (127.0.0.1).
	Escape character is '^]'.
	OpenSM Console connection already in use
	   kill other session (y/n)? y
	OpenSM $

	The connection #1 is forced to quit as it was supposed to, however, now
	the "quit" command will not work in connection #2 and no other users
	can connect!

	It was found that the osm_console_exit and console_close functions had
	been intertwined in an unintended fashion.  osm_console_exit was
	intended to be the opposite of osm_console_init (which closes not only
	any open connections but also the socket).  console_close was intended
	to be used to close a single connection but not the socket.

This patch fixes all this by removing console_close, making cio_close close
only the connection, and fixing osm_console_exit to properly clean up from
osm_console_init.

Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
---
 opensm/include/opensm/osm_console_io.h |    2 +-
 opensm/opensm/osm_console.c            |    6 +++---
 opensm/opensm/osm_console_io.c         |   32 ++++++++++----------------------
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/opensm/include/opensm/osm_console_io.h b/opensm/include/opensm/osm_console_io.h
index f31d811..d1dbbdd 100644
--- a/opensm/include/opensm/osm_console_io.h
+++ b/opensm/include/opensm/osm_console_io.h
@@ -83,7 +83,7 @@ int is_console_enabled(osm_subn_opt_t *p_opt);
 
 #ifdef ENABLE_OSM_CONSOLE_SOCKET
 int cio_open(osm_console_t * p_oct, int new_fd, osm_log_t * p_log);
-int cio_close(osm_console_t * p_oct);
+int cio_close(osm_console_t * p_oct, osm_log_t * p_log);
 int is_authorized(osm_console_t * p_oct);
 #endif
 
diff --git a/opensm/opensm/osm_console.c b/opensm/opensm/osm_console.c
index 97be931..182c64e 100644
--- a/opensm/opensm/osm_console.c
+++ b/opensm/opensm/osm_console.c
@@ -1197,7 +1197,7 @@ static void perfmgr_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
 
 static void quit_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
 {
-	osm_console_exit(&p_osm->console, &p_osm->log);
+	cio_close(&p_osm->console, &p_osm->log);
 }
 
 static void help_version(FILE * out, int detail)
@@ -1459,7 +1459,7 @@ int osm_console(osm_opensm_t * p_osm)
 				osm_console_prompt(p_oct->out);
 			}
 		} else
-			osm_console_exit(p_oct, p_log);
+			cio_close(p_oct, p_log);
 		if (p_line)
 			free(p_line);
 		return 0;
@@ -1469,7 +1469,7 @@ int osm_console(osm_opensm_t * p_osm)
 #ifdef ENABLE_OSM_CONSOLE_SOCKET
 		/* If we are using a socket, we close the current connection */
 		if (p_oct->socket >= 0) {
-			cio_close(p_oct);
+			cio_close(p_oct, p_log);
 			return 0;
 		}
 #endif
diff --git a/opensm/opensm/osm_console_io.c b/opensm/opensm/osm_console_io.c
index 56a2c98..1aa4648 100644
--- a/opensm/opensm/osm_console_io.c
+++ b/opensm/opensm/osm_console_io.c
@@ -92,10 +92,13 @@ int is_console_enabled(osm_subn_opt_t * p_opt)
 
 
 #ifdef ENABLE_OSM_CONSOLE_SOCKET
-int cio_close(osm_console_t * p_oct)
+int cio_close(osm_console_t * p_oct, osm_log_t * p_log)
 {
 	int rtnval = -1;
 	if (p_oct && p_oct->in_fd > 0) {
+		OSM_LOG(p_log, OSM_LOG_INFO,
+			"Console connection closed: %s (%s)\n",
+			p_oct->client_hn, p_oct->client_ip);
 		rtnval = close(p_oct->in_fd);
 		p_oct->in_fd = -1;
 		p_oct->out_fd = -1;
@@ -105,20 +108,6 @@ int cio_close(osm_console_t * p_oct)
 	return rtnval;
 }
 
-static void console_close(osm_console_t * p_oct, osm_log_t * p_log)
-{
-	if (p_oct->socket > 0 && p_oct->in_fd != -1) {
-		OSM_LOG(p_log, OSM_LOG_INFO,
-			"Console connection closed: %s (%s)\n",
-			p_oct->client_hn, p_oct->client_ip);
-		cio_close(p_oct);
-	}
-	if (p_oct->socket > 0) {
-		close(p_oct->socket);
-		p_oct->socket = -1;
-	}
-}
-
 int cio_open(osm_console_t * p_oct, int new_fd, osm_log_t * p_log)
 {
 	/* returns zero if opened fine, -1 otherwise */
@@ -135,7 +124,7 @@ int cio_open(osm_console_t * p_oct, int new_fd, osm_log_t * p_log)
 		p_line = NULL;
 		n = getline(&p_line, &len, file);
 		if (n > 0 && (p_line[0] == 'y' || p_line[0] == 'Y'))
-			console_close(p_oct, p_log);
+			cio_close(p_oct, p_log);
 		else {
 			OSM_LOG(p_log, OSM_LOG_INFO,
 				"Console connection aborted: %s (%s)\n",
@@ -238,13 +227,12 @@ int osm_console_init(osm_subn_opt_t * opt, osm_console_t * p_oct, osm_log_t * p_
 /* clean up and release resources */
 void osm_console_exit(osm_console_t * p_oct, osm_log_t * p_log)
 {
-	/* currently just close the current connection, not the socket */
 #ifdef ENABLE_OSM_CONSOLE_SOCKET
-	if (p_oct->socket > 0 && p_oct->in_fd != -1) {
-		OSM_LOG(p_log, OSM_LOG_INFO,
-			"Console connection closed: %s (%s)\n",
-			p_oct->client_hn, p_oct->client_ip);
-		cio_close(p_oct);
+	cio_close(p_oct, p_log);
+	if (p_oct->socket > 0) {
+		OSM_LOG(p_log, OSM_LOG_INFO, "Closing console socket\n");
+		close(p_oct->socket);
+		p_oct->socket = -1;
 	}
 #endif
 }
-- 
1.5.4.5