Use writev(2) when sending delivery status to the parent
authorHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
Thu, 29 Jun 2017 18:13:40 +0000 (20:13 +0200)
committerHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
Sat, 16 Sep 2017 13:15:15 +0000 (15:15 +0200)
src/src/deliver.c
src/src/transports/smtp.c

index eac5bf77009f2efb479562d4426b06c418db7cd4..94dabab02a6eaed5188b1b9ffa5d04b72734a199 100644 (file)
@@ -3308,7 +3308,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
 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");
 
 DEBUG(D_deliver) debug_printf("reading pipe for subprocess %d (%s)\n",
   (int)p->pid, eop? "ended" : "not ended");
@@ -4130,44 +4135,43 @@ while (parcount > max)
 
 
 
 
 
 
-
 static void
 rmt_dlv_checked_write(int fd, char id, char subid, void * buf, int size)
 {
 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*/
 
 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,
   {
   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 */
 
 /* 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");
   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",
 
 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;
-if ((ret = write(fd, writebuffer, size)) != size)
-  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "Failed writing transport result to pipe: %s\n",
+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");
 }
 
     ret == -1 ? strerror(errno) : "short write");
 }
 
@@ -8551,7 +8555,7 @@ if (cutthrough.fd >= 0 && cutthrough.callout_hold_only)
     if ((pid = fork()) < 0)
       goto fail;
 
     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()))
       {
       close(pfd[1]);
       if ((pid = fork()))
index 147dfdeaf45b9a885b90751e3b1e121b719cc096..557d650ae3d97b29d0fddea568944dc1244febd8 100644 (file)
@@ -3147,7 +3147,7 @@ else
         else
           sprintf(CS sx.buffer, "%.500s\n", addr->unique);
 
         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 "
         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 "