Use writev(2) when sending delivery status to the parent

diff --git a/src/src/deliver.c b/src/src/deliver.c
index f96f57aea981aac462f4c665c0084033f5188c62..c7a74dfca93ba1b093751fe5beea2e45ac269f48 100644 (file)
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -33,7 +33,7 @@ writex(int fd, const void *buffer, size_t len)
     if (written == -1) return done;
     done += written;
     buffer += written;
-    DEBUG(D_deliver) debug_printf("#2130: written %d (done: %d/%d)\n", written, done, len);
+    DEBUG(D_deliver) debug_printf("written %d (done: %d of %d)\n", written, done, len);
     usleep(1000);
     }
 
@@ -3336,7 +3336,12 @@ Each separate item is written to the pipe in a single write(), and as they are
 all short items, the writes will all be atomic and we should never find
 ourselves in the position of having read an incomplete item. "Short" in this
 case can mean up to about 1K in the case when there is a long error message
-associated with an address. */
+associated with an address.
+
+write(3) [Linux] says that atomic writes are not interleaved with each other.
+Not more.
+
+*/
 
 DEBUG(D_deliver) debug_printf("reading pipe for subprocess %d (%s)\n",
   (int)p->pid, eop? "ended" : "not ended");
@@ -3455,7 +3460,7 @@ while (!done)
     case '_':
       {
         ptr += required - PIPE_HEADER_SIZE;
-        DEBUG(D_deliver) debug_printf("#2130: read %d bytes\n", required);
+        DEBUG(D_deliver) debug_printf("read %d bytes\n", required);
         break;
       }
 #endif
@@ -4171,51 +4176,57 @@ while (parcount > max)
 
 
 
-
 static void
 rmt_dlv_checked_write(int fd, char id, char subid, void * buf, int size)
 {
-uschar writebuffer[PIPE_HEADER_SIZE + BIG_BUFFER_SIZE];
-int header_length;
+uschar pipe_header[PIPE_HEADER_SIZE+1];
+size_t total_len = PIPE_HEADER_SIZE + size;
+
+struct iovec iov[2] = {
+  { pipe_header, PIPE_HEADER_SIZE },  /* indication about the data to expect */
+  { buf, size }                       /* *the* data */
+};
+
 int ret;
 
 /* we assume that size can't get larger then BIG_BUFFER_SIZE which currently is set to 16k */
 /* complain to log if someone tries with buffer sizes we can't handle*/
 
-if (size > 99999)
+if (size > BIG_BUFFER_SIZE-1)
   {
   log_write(0, LOG_MAIN|LOG_PANIC_DIE,
-    "Failed writing transport result to pipe: can't handle buffers > 99999 bytes. truncating!\n");
-  size = 99999;
+    "Failed writing transport result to pipe: can't handle buffers > %d bytes. truncating!\n",
+      BIG_BUFFER_SIZE-1);
+  size = BIG_BUFFER_SIZE;
   }
 
-/* to keep the write() atomic we build header in writebuffer and copy buf behind */
-/* two write() calls would increase the complexity of reading from pipe */
+/* Should we check that we do not write more than PIPE_BUF? What whould
+that help? */
 
 /* convert size to human readable string prepended by id and subid */
-header_length = snprintf(CS writebuffer, PIPE_HEADER_SIZE+1, "%c%c%05d", id, subid, size);
-if (header_length != PIPE_HEADER_SIZE)
-  {
+if (PIPE_HEADER_SIZE != snprintf(CS pipe_header, PIPE_HEADER_SIZE+1, "%c%c%05d", id, subid, size))
   log_write(0, LOG_MAIN|LOG_PANIC_DIE, "header snprintf failed\n");
-  writebuffer[0] = '\0';
-  }
 
 DEBUG(D_deliver) debug_printf("header write id:%c,subid:%c,size:%d,final:%s\n",
-                                 id, subid, size, writebuffer);
-
-if (buf && size > 0)
-  memcpy(writebuffer + PIPE_HEADER_SIZE, buf, size);
+                                 id, subid, size, pipe_header);
 
-size += PIPE_HEADER_SIZE;
 #ifdef HS12_TRIGGER_2130
-if ((ret = writex(fd, writebuffer, size)) != size)
+for (int i = 0; i < 2; i++)
+  {
+  ret = writex(fd, iov[i].iov_base, iov[i].iov_len);
+  DEBUG(D_deliver) debug_printf("written %d for pipe%s\n", ret, i == 0 ? "header" : "data");
+
+  if (ret != iov[i].iov_len)
+    log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result to pipe (%d of %d bytes): %s",
+      ret == -1 ? 0 : ret, total_len,
+      ret == -1 ? strerror(errno) : "short write");
+  }
 #else
-if ((ret = write(fd, writebuffer, size)) != size)
-#endif
-  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result "
-    "to pipe after %d of %d bytes: %s\n",
-    ret, size,
+if ((ret = writev(fd, iov, 2)) != total_len)
+  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result to pipe (%d of %d bytes): %s",
+    ret == -1 ? 0 : ret, total_len,
     ret == -1 ? strerror(errno) : "short write");
+#endif
 
 }
 
@@ -8608,7 +8619,7 @@ if (cutthrough.fd >= 0 && cutthrough.callout_hold_only)
     if ((pid = fork()) < 0)
       goto fail;
 
-    else if (pid == 0)         /* child: fork again to totally dosconnect */
+    else if (pid == 0)         /* child: fork again to totally disconnect */
       {
       close(pfd[1]);
       if ((pid = fork()))

diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index 147dfdeaf45b9a885b90751e3b1e121b719cc096..557d650ae3d97b29d0fddea568944dc1244febd8 100644 (file)
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -3147,7 +3147,7 @@ else
         else
           sprintf(CS sx.buffer, "%.500s\n", addr->unique);
 
-        DEBUG(D_deliver) debug_printf("S:journalling %s\n", sx.buffer);
+        DEBUG(D_deliver) debug_printf("S:journalling %s", sx.buffer);
         len = Ustrlen(CS sx.buffer);
         if (write(journal_fd, sx.buffer, len) != len)
           log_write(0, LOG_MAIN|LOG_PANIC, "failed to write journal for "