Fix critical bugs: signal safety, packet parsing, fd leaks

- Signal handler (intHandler): move non-async-signal-safe calls
  (LOG, fork, sleep, stopPacketLogger, run_aqualinkd_upgrade) out of
  the handler and into the post-main-loop cleanup path.  The handler
  now only sets volatile flags and optionally closes the blocking fd.

- Packet parsing (fix_packet): change loop bound from i <= packet_length
  to i < packet_length - 1 to prevent buffer over-read when checking
  packet_buffer[i+1].

- Pentair packet construction (send_pentair_command): replace pre-increment
  packet[++i] pattern with direct indexing to eliminate a gap byte and
  extra trailing NUL in sent packets.

- Serial port init (_init_serial_port): add close(fd) on all error paths
  after open() succeeds to prevent file descriptor leaks.  The tcsetattr
  failure path also calls unlock_port(fd) since the port was locked.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

source/aq_serial.c

@@ -599,11 +599,13 @@ int _init_serial_port(const char* tty, bool blocking, bool readahead)
 
   if (tcgetattr(fd, &newtio) != 0) {
     LOG(RSSD_LOG,LOG_ERR, "Unable to get port attributes: %s, error %d\n", tty,errno);
+    close(fd);
     return -1;
   }
 
   if ( lock_port(fd, tty) < 0) {
     //LOG(RSSD_LOG,LOG_ERR, "Unable to lock port: %s, error %d\n", tty, errno);
+    close(fd);
     return -1;
   }
 
@@ -656,6 +658,8 @@ int _init_serial_port(const char* tty, bool blocking, bool readahead)
   tcflush(fd, TCIFLUSH);
   if (tcsetattr(fd, TCSANOW, &newtio) != 0) {
     LOG(RSSD_LOG,LOG_ERR, "Unable to set port attributes: %s, error %d\n", tty,errno);
+    unlock_port(fd);
+    close(fd);
     return -1;
   }
 
@@ -755,14 +759,12 @@ void send_pentair_command(int fd, unsigned char *packet_buffer, int size)
     //packet[i] = packet_buffer[i-4];    
   }
 
-  packet[++i] = NUL;  // Checksum
-  packet[++i] = NUL;  // Checksum
-  generate_pentair_checksum(&packet[1], i);
-  //packet[++i] = NUL;
+  // Place checksum placeholders immediately after data (no gap byte)
+  packet[i] = NUL;    // Checksum high
+  packet[i+1] = NUL;  // Checksum low
+  generate_pentair_checksum(&packet[1], i+2);
 
-
-  //logPacket(packet, i);
-  send_packet(fd,packet,++i);
+  send_packet(fd, packet, i+2);
 }
 
 #ifndef SEND_CMD_WITH_TRAILING_NUL
@@ -1077,7 +1079,7 @@ int fix_packet(unsigned char *packet_buffer, int packet_length, bool getCached)
   // Ignore the first two bytes, assuming they are 0x10, 0x02
   
 
-  for (int i=2; i <= packet_length; i++ ) {
+  for (int i=2; i < packet_length - 1; i++ ) {
     if (packet_buffer[i] == DLE && packet_buffer[i+1] == STX) {
       LOG(RSSD_LOG,LOG_DEBUG, "Packet Fix: found start in middle of frame\n");
       int p1_length = i;

source/aqualinkd.c

@@ -72,6 +72,8 @@
 
 static volatile bool _keepRunning = true;
 static volatile bool _restart = false;
+static volatile sig_atomic_t _received_signal = 0;
+static volatile sig_atomic_t _upgrade_requested = 0;
 //char** _argv;
 //static struct aqconfig _aqconfig_;
 static struct aqualinkdata _aqualink_data;
@@ -103,49 +105,25 @@ bool isAqualinkDStopping() {
 
 void intHandler(int sig_num)
 {
-  if (sig_num == SIGRUPGRADE) {
-    if (! run_aqualinkd_upgrade(false)) {
-      LOG(AQUA_LOG,LOG_ERR, "AqualinkD upgrade failed!\n");
-    }
-    return; // Let the upgrade process terminate us.
-  }
+  // Only perform async-signal-safe operations here:
+  // set flags and let the main loop handle cleanup.
+  _received_signal = sig_num;
 
-  LOG(AQUA_LOG,LOG_WARNING, "Stopping!\n");
+  if (sig_num == SIGRUPGRADE) {
+    _upgrade_requested = 1;
+    return;
+  }
 
   _keepRunning = false;
 
   if (sig_num == SIGRESTART) {
-    LOG(AQUA_LOG,LOG_WARNING, "Restarting AqualinkD!\n");
-    // If we are deamonized, we need to use the system
-    if (_aqconfig_.deamonize) {
-      if(fork() == 0) {
-        sleep(2);
-        char *newargv[] = {"/bin/systemctl", "restart", "aqualinkd", NULL};
-        char *newenviron[] = { NULL };
-        execve(newargv[0], newargv, newenviron);
-        exit (EXIT_SUCCESS);
-      }
-    } else {
-      _restart = true;
-    }
+    _restart = true;
   }
-  //LOG(AQUA_LOG,LOG_NOTICE, "Stopping!\n");
-  //if (dummy){}// stop compile warnings
 
-  // In blocking mode, die as cleanly as possible.
-#ifdef AQ_NO_THREAD_NETSERVICE
-  if (_aqconfig_.rs_poll_speed < 0) {
-    stopPacketLogger();
-    // This should force port to close and do somewhat gracefull exit.
-    close_blocking_serial_port();
-    //exit(-1);
-  }
-#else
-  stopPacketLogger();
-  // This should force port to close and do somewhat gracefull exit.
+  // In blocking mode, force the serial port closed so the blocking
+  // read returns and the main loop can exit.  close() is async-signal-safe.
   if (serial_blockingmode())
     close_blocking_serial_port();
-#endif
 }
 
 bool isVirtualButtonEnabled() {
@@ -1531,6 +1509,32 @@ void main_loop()
     //delay(10);
   }
   
+  // Handle deferred signal actions (moved out of signal handler for async-signal-safety).
+  if (_received_signal != 0) {
+    LOG(AQUA_LOG,LOG_WARNING, "Received signal %d, stopping!\n", _received_signal);
+  }
+
+  if (_upgrade_requested) {
+    _upgrade_requested = 0;
+    if (! run_aqualinkd_upgrade(false)) {
+      LOG(AQUA_LOG,LOG_ERR, "AqualinkD upgrade failed!\n");
+    }
+    // Let the upgrade process terminate us.
+  }
+
+  if (_restart && _aqconfig_.deamonize) {
+    // When daemonized, fork a child to restart via systemctl
+    LOG(AQUA_LOG,LOG_WARNING, "Restarting AqualinkD (daemonized)!\n");
+    if(fork() == 0) {
+      sleep(2);
+      char *newargv[] = {"/bin/systemctl", "restart", "aqualinkd", NULL};
+      char *newenviron[] = { NULL };
+      execve(newargv[0], newargv, newenviron);
+      exit(EXIT_SUCCESS);
+    }
+    _restart = false; // Let the parent exit normally
+  }
+
   //if (_aqconfig_.debug_RSProtocol_packets) stopPacketLogger();
   stopPacketLogger();