From 99d4d904b23d49ef4d17950b82608c6049e349e2 Mon Sep 17 00:00:00 2001 From: wb2osz Date: Sun, 11 Feb 2018 19:45:44 -0500 Subject: [PATCH] KISS over TCP behaved strangely with multiple client apps attached. --- aclients.c | 21 +++--------- audio_win.c | 2 +- direwolf.h | 12 +++++++ igate.c | 10 ++---- kiss.c | 4 +-- kissnet.c | 92 ++++++++++++----------------------------------------- kissutil.c | 9 ++---- server.c | 34 +++++++------------- ttcalc.c | 31 ++++-------------- 9 files changed, 63 insertions(+), 152 deletions(-) diff --git a/aclients.c b/aclients.c index 28b7cf3..6675b09 100644 --- a/aclients.c +++ b/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); diff --git a/audio_win.c b/audio_win.c index c96a00a..4af25ce 100644 --- a/audio_win.c +++ b/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()); diff --git a/direwolf.h b/direwolf.h index 5c5c295..04f840b 100644 --- a/direwolf.h +++ b/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. */ diff --git a/igate.c b/igate.c index af2bbda..1d73c19 100644 --- a/igate.c +++ b/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 diff --git a/kiss.c b/kiss.c index 761f6ba..8186df6 100644 --- a/kiss.c +++ b/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"); } diff --git a/kissnet.c b/kissnet.c index cd8c5e1..ee58c43 100644 --- a/kissnet.c +++ b/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; -#if DEBUG - text_color_set(DW_COLOR_DEBUG); - dw_printf ("kissnet_listen_thread ( socket = %d )\n", client_sock); -#endif int client = (int)(long)arg; +#if DEBUG + text_color_set(DW_COLOR_DEBUG); + dw_printf ("kissnet_listen_thread ( client = %d, socket fd = %d )\n", client, client_sock[client]); +#endif + assert (client >= 0 && client < MAX_NET_CLIENTS); diff --git a/kissutil.c b/kissutil.c index d7bd11e..66a7c15 100644 --- a/kissutil.c +++ b/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. diff --git a/server.c b/server.c index 243b351..0a74ff3 100644 --- a/server.c +++ b/server.c @@ -491,14 +491,16 @@ void server_init (struct audio_s *audio_config_p, struct misc_config_s *mc) cmd_listen_th[client] = (HANDLE)_beginthreadex (NULL, 0, cmd_listen_thread, (void*)client, 0, NULL); if (cmd_listen_th[client] == NULL) { text_color_set(DW_COLOR_ERROR); - dw_printf ("Could not create AGW command listening thread\n"); + dw_printf ("Could not create AGW command listening thread for client %d\n", client); return; } #else e = pthread_create (&cmd_listen_tid[client], NULL, cmd_listen_thread, (void *)(long)client); if (e != 0) { text_color_set(DW_COLOR_ERROR); - perror("Could not create AGW command listening thread"); + dw_printf ("Could not create AGW command listening thread for client %d\n", client); + // Replace add perror with better message handling. + perror(""); return; } #endif @@ -816,7 +818,7 @@ void server_send_rec_packet (int chan, packet_t pp, unsigned char *fbuf, int fl } #if __WIN32__ - err = send (client_sock[client], (char*)(&agwpe_msg), sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE), 0); + err = SOCK_SEND (client_sock[client], (char*)(&agwpe_msg), sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); if (err == SOCKET_ERROR) { text_color_set(DW_COLOR_ERROR); @@ -827,7 +829,7 @@ void server_send_rec_packet (int chan, packet_t pp, unsigned char *fbuf, int fl dlq_client_cleanup (client); } #else - err = write (client_sock[client], &agwpe_msg, sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); + err = SOCK_SEND (client_sock[client], &agwpe_msg, sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); if (err <= 0) { text_color_set(DW_COLOR_ERROR); @@ -920,8 +922,8 @@ void server_send_rec_packet (int chan, packet_t pp, unsigned char *fbuf, int fl debug_print (TO_CLIENT, client, &agwpe_msg.hdr, sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); } -#if __WIN32__ - err = send (client_sock[client], (char*)(&agwpe_msg), sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE), 0); +#if __WIN32__ + err = SOCK_SEND (client_sock[client], (char*)(&agwpe_msg), sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); if (err == SOCKET_ERROR) { text_color_set(DW_COLOR_ERROR); @@ -932,7 +934,7 @@ void server_send_rec_packet (int chan, packet_t pp, unsigned char *fbuf, int fl dlq_client_cleanup (client); } #else - err = write (client_sock[client], &agwpe_msg, sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); + err = SOCK_SEND (client_sock[client], &agwpe_msg, sizeof(agwpe_msg.hdr) + netle2host(agwpe_msg.hdr.data_len_NETLE)); if (err <= 0) { text_color_set(DW_COLOR_ERROR); @@ -1145,14 +1147,7 @@ static int read_from_socket (int fd, char *ptr, int len) while (got_bytes < len) { int n; -#if __WIN32__ - -//TODO: any flags for send/recv? - - n = recv (fd, ptr + got_bytes, len - got_bytes, 0); -#else - n = read (fd, ptr + got_bytes, len - got_bytes); -#endif + n = SOCK_RECV (fd, ptr + got_bytes, len - got_bytes); #if DEBUG text_color_set(DW_COLOR_DEBUG); @@ -1195,10 +1190,7 @@ static void send_to_client (int client, void *reply_p) { struct agwpe_s *ph; int len; -#if __WIN32__ -#else int err; -#endif ph = (struct agwpe_s *) reply_p; // Replies are often hdr + other stuff. @@ -1216,12 +1208,8 @@ static void send_to_client (int client, void *reply_p) debug_print (TO_CLIENT, client, ph, len); } -#if __WIN32__ - send (client_sock[client], (char*)(ph), len, 0); -#else - err = write (client_sock[client], ph, len); + err = SOCK_SEND (client_sock[client], (char*)(ph), len); (void)err; -#endif } diff --git a/ttcalc.c b/ttcalc.c index ccebf89..9c5b4e4 100644 --- a/ttcalc.c +++ b/ttcalc.c @@ -1,4 +1,3 @@ - // // This file is part of Dire Wolf, an amateur radio packet TNC. // @@ -108,11 +107,10 @@ int main (int argc, char *argv[]) char data[1024]; char hostname[30] = "localhost"; char port[10] = "8000"; + int err; #if __WIN32__ #else - int err; - setlinebuf (stdout); #endif @@ -142,13 +140,8 @@ int main (int argc, char *argv[]) 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)); + err = SOCK_SEND (server_sock, (char*)(&mon_cmd), sizeof(mon_cmd)); (void)err; -#endif - /* * Print all of the monitored packets. @@ -157,11 +150,7 @@ int main (int argc, char *argv[]) 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, received %d command bytes.\n", n); @@ -171,14 +160,10 @@ int main (int argc, char *argv[]) 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 received %d data bytes when %d expected.\n", n, mon_cmd.data_len); + printf ("Read error, client received %d data bytes when %d expected. Terminating.\n", n, mon_cmd.data_len); exit (1); } } @@ -254,11 +239,7 @@ int main (int argc, char *argv[]) xmit_raw.hdr.kind_lo = 'K'; xmit_raw.hdr.data_len = 1 + ax25_pack (reply_pp, xmit_raw.frame); -#if __WIN32__ - send (server_sock, (char*)(&xmit_raw), sizeof(xmit_raw.hdr)+xmit_raw.hdr.data_len, 0); -#else - err = write (server_sock, (char*)(&xmit_raw), sizeof(xmit_raw.hdr)+xmit_raw.hdr.data_len); -#endif + err = SOCK_SEND (server_sock, (char*)(&xmit_raw), sizeof(xmit_raw.hdr)+xmit_raw.hdr.data_len); ax25_delete (reply_pp); }