[PATCH] another old API removed: packet_close()

[Markus Schmidt]

Here is another small patch to remove an old opacket API call.


packet_close() simply calls ssh_packet_close() with the current ssh 
pointer as a parameter and then clears the active_state pointer (which 
in itself is just a copy of the master ssh pointer and which is used 
only for these the old apis).

There are only two instances of packet_close(), one in ssh.c and one in 
sshd.c.  There we can simply replace the packet_close() call with

	ssh_packet_close(ssh);
	active_state = NULL; /* XXX legacy API compat */

Additionally, I found something related in ssh.c.  There is a bogus 
assignment between the ssh pointer and active_state which I think should 
be removed, because further up in ssh.c, we set active_state to the 
value of ssh (I understand ssh is the master pointer and active_state 
ist just a copy for use with the old opacket api calls).

Hence, doing this later appears to be bogus and should be removed:

-	ssh = active_state; /* XXX */


And finally, at the end of main(), before exiting main ssh.c leaks the 
memory on the ssh pointer (it is allocated by never freed).  The leak is 
inconsequential, but I think it should be freed anyway.

Hence, after calling ssh_packet_close() we should add:

+	free(ssh);
+	ssh= NULL;



PATCH:

  opacket.c | 7 -------
  opacket.h | 1 -
  ssh.c     | 8 +++++---
  sshd.c    | 5 ++++-
  4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/opacket.c b/opacket.c
index bff4c36..af8516c 100755
--- a/opacket.c
+++ b/opacket.c
@@ -237,13 +237,6 @@ packet_read_poll_seqnr(u_int32_t *seqnr)
  }

  void
-packet_close(void)
-{
-	ssh_packet_close(active_state);
-	active_state = NULL;
-}
-
-void
  packet_process_incoming(const char *buf, u_int len)
  {
  	int r;
diff --git a/opacket.h b/opacket.h
index d711468..067de68 100755
--- a/opacket.h
+++ b/opacket.h
@@ -35,7 +35,6 @@ do { \
  } while (0)

  /* old API */
-void	 packet_close(void);
  u_int	 packet_get_char(void);
  u_int	 packet_get_int(void);
  int	 packet_read_seqnr(u_int32_t *);
diff --git a/ssh.c b/ssh.c
index 3b5ea99..48f347c 100755
--- a/ssh.c
+++ b/ssh.c
@@ -1348,8 +1348,6 @@ main(int ac, char **av)
  	packet_set_timeout(options.server_alive_interval,
  	    options.server_alive_count_max);

-	ssh = active_state; /* XXX */
-
  	if (timeout_ms > 0)
  		debug3("timeout: %d ms remain after connect", timeout_ms);

@@ -1491,7 +1489,11 @@ main(int ac, char **av)

   skip_connect:
  	exit_status = ssh_session2(ssh, pw);
-	packet_close();
+	ssh_packet_close(ssh);
+	active_state = NULL; /* XXX legacy API compat */
+
+	free(ssh);
+	ssh= NULL;

  	if (options.control_path != NULL && muxserver_sock != -1)
  		unlink(options.control_path);
diff --git a/sshd.c b/sshd.c
index 65b96d4..760593b 100755
--- a/sshd.c
+++ b/sshd.c
@@ -2043,7 +2043,10 @@ main(int ac, char **av)
  	    (unsigned long long)obytes, (unsigned long long)ibytes);

  	verbose("Closing connection to %.500s port %d", remote_ip, remote_port);
-	packet_close();
+	ssh_packet_close(ssh);
+	active_state = NULL; /* XXX legacy API compat */
+	free(ssh);
+	ssh= NULL;

  	if (use_privsep)
  		mm_terminate();