KISS over TCP behaved strangely with multiple client apps attached.

Author: wb2osz

aclients.c

@@ -549,12 +549,7 @@ static void * client_thread_net (void *arg)
 
 	mon_cmd.kind_lo = 'k';
 
-#if __WIN32__	      
-	send (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd), 0);
-#else
-	err = write (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd));
-#endif
-
+	SOCK_SEND (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd));
 
 /*
  * Print all of the monitored packets.
@@ -563,14 +558,10 @@ static void * client_thread_net (void *arg)
 	while (1) {
 	  int n;
 
-#if __WIN32__
-	  n = recv (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd), 0);
-#else
-	  n = read (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd));
-#endif
+	  n = SOCK_RECV (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd));
 
 	  if (n != sizeof(mon_cmd)) {
-	    printf ("Read error, client %d received %d command bytes.\n", my_index, n);
+	    printf ("Read error, client %d received %d command bytes.  Terminating.\n", my_index, n);
 	    exit (1);
 	  }
 
@@ -581,11 +572,7 @@ static void * client_thread_net (void *arg)
 	  assert (mon_cmd.data_len >= 0 && mon_cmd.data_len < (int)(sizeof(data)));
 
 	  if (mon_cmd.data_len > 0) {
-#if __WIN32__
-	    n = recv (server_sock, data, mon_cmd.data_len, 0);
-#else
-	    n = read (server_sock, data, mon_cmd.data_len);
-#endif
+	    n = SOCK_RECV (server_sock, data, mon_cmd.data_len);
 
 	    if (n != mon_cmd.data_len) {
 	      printf ("Read error, client %d received %d data bytes.\n", my_index, n);

audio_win.c

@@ -850,7 +850,7 @@ int audio_get (int a)
 
               assert (A->udp_sock > 0);
 
-	      res = recv (A->udp_sock, A->stream_data, SDR_UDP_BUF_MAXLEN, 0);
+	      res = SOCK_RECV (A->udp_sock, A->stream_data, SDR_UDP_BUF_MAXLEN);
 	      if (res <= 0) {
 	        text_color_set(DW_COLOR_ERROR);
 	        dw_printf ("Can't read from udp socket, errno %d", WSAGetLastError());

direwolf.h

@@ -249,6 +249,18 @@ typedef pthread_mutex_t dw_mutex_t;
 
 
 
+// Formerly used write/read on Linux, for some forgotten reason,
+// but always using send/recv makes more sense.
+// Need option to prevent a SIGPIPE signal on Linux.  (added for 1.5 beta 2)
+
+#if __WIN32__
+#define SOCK_SEND(s,data,size) send(s,data,size,0)
+#else
+#define SOCK_SEND(s,data,size) send(s,data,size, MSG_NOSIGNAL)
+#endif
+#define SOCK_RECV(s,data,size) recv(s,data,size,0)
+
+
 /* Platform differences for string functions. */
 
 

igate.c

@@ -1266,7 +1266,7 @@ static void send_msg_to_server (const char *imsg, int imsg_len)
 
 
 #if __WIN32__	
-        err = send (igate_sock, stemp, stemp_len, 0);
+        err = SOCK_SEND (igate_sock, stemp, stemp_len);
 	if (err == SOCKET_ERROR)
 	{
 	  text_color_set(DW_COLOR_ERROR);
@@ -1277,7 +1277,7 @@ static void send_msg_to_server (const char *imsg, int imsg_len)
 	  WSACleanup();
 	}
 #else
-        err = write (igate_sock, stemp, stemp_len);
+        err = SOCK_SEND (igate_sock, stemp, stemp_len);
 	if (err <= 0)
 	{
 	  text_color_set(DW_COLOR_ERROR);
@@ -1319,11 +1319,7 @@ static int get1ch (void)
 	  // TODO: might read complete packets and unpack from own buffer
 	  // rather than using a system call for each byte.
 
-#if __WIN32__
-	  n = recv (igate_sock, (char*)(&ch), 1, 0);
-#else
-	  n = read (igate_sock, &ch, 1);
-#endif
+	  n = SOCK_RECV (igate_sock, (char*)(&ch), 1);
 
 	  if (n == 1) {
 #if DEBUG9

kiss.c

@@ -220,8 +220,8 @@ void kisspt_init (struct misc_config_s *mc)
 	  }
 	}
 	else {
-	  text_color_set(DW_COLOR_INFO);
-	  dw_printf ("Use -p command line option to enable KISS pseudo terminal.\n");
+	  //text_color_set(DW_COLOR_INFO);
+	  //dw_printf ("Use -p command line option to enable KISS pseudo terminal.\n");
 	}
 
 

kissnet.c

@@ -208,7 +208,7 @@ void kissnet_init (struct misc_config_s *mc)
 	pthread_t cmd_listen_tid[MAX_NET_CLIENTS];
 	int e;
 #endif
-	int kiss_port = mc->kiss_port;
+	int kiss_port = mc->kiss_port;		/* default 8001 but easily changed. */
 
 
 #if DEBUG
@@ -258,14 +258,16 @@ void kissnet_init (struct misc_config_s *mc)
 	  cmd_listen_th[client] = (HANDLE)_beginthreadex (NULL, 0, kissnet_listen_thread, (void*)client, 0, NULL);
 	  if (cmd_listen_th[client] == NULL) {
 	    text_color_set(DW_COLOR_ERROR);
-	    dw_printf ("Could not create KISS socket command listening thread\n");
+	    dw_printf ("Could not create KISS command listening thread for client %d\n", client);
 	    return;
 	  }
 #else
-	  e = pthread_create (&(cmd_listen_tid[client]), NULL, kissnet_listen_thread, NULL);
+	  e = pthread_create (&(cmd_listen_tid[client]), NULL, kissnet_listen_thread, (void *)(long)client);
 	  if (e != 0) {
 	    text_color_set(DW_COLOR_ERROR);
-	    perror("Could not create KISS socket command listening thread");
+	    dw_printf ("Could not create KISS command listening thread for client %d\n", client);
+	    // Replace add perror with better message handling.
+	    perror("");
 	    return;
 	  }
 #endif
@@ -589,6 +591,10 @@ void kissnet_send_rec_packet (int chan, int kiss_cmd, unsigned char *fbuf, int f
 	      text_color_set(DW_COLOR_ERROR);
 	      dw_printf ("KISS TCP: Something unexpected from client application.\n");
 	      dw_printf ("Is client app treating this like an old TNC with command mode?\n");
+	      dw_printf ("This can be caused by the application sending commands to put a\n");
+	      dw_printf ("traditional TNC into KISS mode.  It is usually a harmless warning.\n");
+	      dw_printf ("For best results, configure for a KISS-only TNC to avoid this.\n");
+	      dw_printf ("In the case of APRSISCE/32, use \"Simply(KISS)\" rather than \"KISS.\"\n");
 
 	      flen = strlen((char*)fbuf);
 	      if (kiss_debug) {
@@ -623,21 +629,21 @@ void kissnet_send_rec_packet (int chan, int kiss_cmd, unsigned char *fbuf, int f
 	    }
 
 #if __WIN32__	
-            err = send (client_sock[client], (char*)kiss_buff, kiss_len, 0);
+            err = SOCK_SEND(client_sock[client], (char*)kiss_buff, kiss_len);
 	    if (err == SOCKET_ERROR)
 	    {
 	      text_color_set(DW_COLOR_ERROR);
-	      dw_printf ("\nError %d sending message to KISS client application.  Closing connection.\n\n", WSAGetLastError());
+	      dw_printf ("\nError %d sending message to KISS client %d application.  Closing connection.\n\n", WSAGetLastError(), client);
 	      closesocket (client_sock[client]);
 	      client_sock[client] = -1;
 	      WSACleanup();
 	    }
 #else
-            err = write (client_sock[client], kiss_buff, kiss_len);
+            err = SOCK_SEND (client_sock[client], kiss_buff, kiss_len);
 	    if (err <= 0)
 	    {
 	      text_color_set(DW_COLOR_ERROR);
-	      dw_printf ("\nError sending message to KISS client application.  Closing connection.\n\n");
+	      dw_printf ("\nError sending message to KISS client %d application.  Closing connection.\n\n", client);
 	      close (client_sock[client]);
 	      client_sock[client] = -1;
 	    }
@@ -649,63 +655,6 @@ void kissnet_send_rec_packet (int chan, int kiss_cmd, unsigned char *fbuf, int f
 
 
 
-/*-------------------------------------------------------------------
- *
- * Name:        read_from_socket
- *
- * Purpose:     Read from socket until we have desired number of bytes.
- *
- * Inputs:	fd		- file descriptor.
- *		ptr		- address where data should be placed.
- *		len		- desired number of bytes.
- *
- * Description:	Just a wrapper for the "read" system call but it should
- *		never return fewer than the desired number of bytes.
- *
- * 		Not really needed for KISS because we are dealing with
- *		a stream of bytes rather than message blocks.
- *
- *--------------------------------------------------------------------*/
-
-static int read_from_socket (int fd, char *ptr, int len)
-{
-	int got_bytes = 0;
-
-#if DEBUG
-	text_color_set(DW_COLOR_DEBUG);
-	dw_printf ("read_from_socket (%d, %p, %d)\n", fd, ptr, len);
-#endif
-	while (got_bytes < len) {
-	  int n;
-
-#if __WIN32__
-	  n = recv (fd, ptr + got_bytes, len - got_bytes, 0);
-#else
-	  n = read (fd, ptr + got_bytes, len - got_bytes);
-#endif
-
-//Would be useful to have more detailed explanation from the error code.
-
-#if DEBUG
-	  text_color_set(DW_COLOR_DEBUG);
-	  dw_printf ("read_from_socket: n = %d\n", n);
-#endif
-	  if (n <= 0) {
-	    return (n);
-	  }
-
-	  got_bytes += n;
-	}
-	assert (got_bytes >= 0 && got_bytes <= len);
-
-#if DEBUG
-	text_color_set(DW_COLOR_DEBUG);
-	dw_printf ("read_from_socket: return %d\n", got_bytes);
-#endif
-	return (got_bytes);
-}
-
-
 /*-------------------------------------------------------------------
  *
  * Name:        kissnet_listen_thread
@@ -739,7 +688,7 @@ static int kiss_get (int client)
 
 	  /* Just get one byte at a time. */
 
-	  n = read_from_socket (client_sock[client], (char *)(&ch), 1);
+	  n = SOCK_RECV (client_sock[client], (char *)(&ch), 1);
 
 	  if (n == 1) {
 #if DEBUG9
@@ -759,7 +708,7 @@ static int kiss_get (int client)
 	  }
 
           text_color_set(DW_COLOR_ERROR);
-	  dw_printf ("\nError reading KISS byte from client application %d.  Closing connection.\n\n", client);
+	  dw_printf ("\nKISS client application %d has gone away.\n\n", client);
 #if __WIN32__
 	  closesocket (client_sock[client]);
 #else
@@ -775,13 +724,14 @@ static THREAD_F kissnet_listen_thread (void *arg)
 {
 	unsigned char ch;
 
+
+	int client = (int)(long)arg;
+
 #if DEBUG
 	text_color_set(DW_COLOR_DEBUG);
-	dw_printf ("kissnet_listen_thread ( socket = %d )\n", client_sock);
+	dw_printf ("kissnet_listen_thread ( client = %d, socket fd = %d )\n", client, client_sock[client]);
 #endif
 
-	int client = (int)(long)arg;
-
 	assert (client >= 0 && client < MAX_NET_CLIENTS);
 
 

kissutil.c

@@ -90,12 +90,6 @@ static THREAD_F tnc_listen_serial (void *arg);
 static void send_to_kiss_tnc (int chan, int cmd, char *data, int dlen);
 static void hex_dump (unsigned char *p, int len);
 
-// Formerly used write/read on Linux, for some forgotten reason,
-// but making them the same seems to make more sense.
-#define SOCK_SEND(s,data,size) send(s,data,size,0)
-#define SOCK_RECV(s,data,size) recv(s,data,size,0)
-
-
 static void usage(void);
 static void usage2(void);
 
@@ -809,6 +803,9 @@ void kiss_process_msg (unsigned char *kiss_msg, int kiss_len, int debug, int cli
 
 	      ax25_safe_print ((char *)pinfo, info_len, 0);
 	      dw_printf ("\n");
+#if __WIN32__
+	      fflush (stdout);
+#endif
 
 /*
  * Add to receive queue directory if specified.