Queuefile: avoid using buffered I/O - no point for a block-copy

and it meant (an admittedly ingnorable) Coverity whine about a FILE leak

Take the oppurtunity to constify a utility function

diff --git a/src/src/functions.h b/src/src/functions.h
index 9b46c953d6fe23efd01ca1dcb2f05f08431873b8..dad00c971f0d4e9fa1834fcce0802c05f2798569 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -415,7 +415,7 @@ extern FILE   *spool_mbox(unsigned long *, const uschar *);
 #endif
 extern BOOL    spool_move_message(uschar *, uschar *, uschar *, uschar *);
 extern uschar *spool_dname(const uschar *, uschar *);
-extern uschar *spool_fname(const uschar *, uschar *, uschar *, uschar *);
+extern uschar *spool_fname(const uschar *, const uschar *, const uschar *, const uschar *);
 extern uschar *spool_sname(const uschar *, uschar *);
 extern int     spool_open_datafile(uschar *);
 extern int     spool_open_temp(uschar *);

diff --git a/src/src/queue.c b/src/src/queue.c
index eddb8d85c6a9bd667d89e2e271423a179d5dd331..551c32dd14ea77893bff7565b341476166ea8bcd 100644 (file)
--- a/src/src/queue.c
+++ b/src/src/queue.c
@@ -39,7 +39,8 @@ return string_sprintf("%s%s%s%s%s",
 }
 
 uschar *
-spool_fname(const uschar * purpose, uschar * subdir, uschar * fname, uschar * suffix)
+spool_fname(const uschar * purpose, const uschar * subdir, const uschar * fname,
+               const uschar * suffix)
 {
 return string_sprintf("%s/%s/%s/%s/%s%s",
        spool_directory, queue_name, purpose, subdir, fname, suffix);

diff --git a/src/src/transports/queuefile.c b/src/src/transports/queuefile.c
index 79178ef0cd0edeabee56cefee73f6fe0f6c60375..25747b3ab9cb2411a1b69cec5b7ec7f9fdc0d3eb 100644 (file)
--- a/src/src/transports/queuefile.c
+++ b/src/src/transports/queuefile.c
@@ -49,24 +49,29 @@ if (!ob->dirname)
 /* This function will copy from a file to another
 
 Arguments:
-  to_fd        FILE to write to (the destination queue file)
-  from_fd      FILE to read from (the spool queue file)
+  dst        fd to write to (the destination queue file)
+  src        fd to read from (the spool queue file)
 
-Returns:       TRUE if all went well, FALSE otherwise
+Returns:       TRUE if all went well, FALSE otherwise with errno set
 */
 
 static BOOL
-copy_spool_file (FILE *to_fd, FILE *from_fd)
+copy_spool_file(int dst, int src)
 {
 int i, j;
 uschar buffer[16384];
+uschar * s;
 
-rewind(from_fd);
+if (lseek(src, 0, SEEK_SET) != 0)
+  return FALSE;
 
 do
-  if ((j = fread(buffer, 1, sizeof(buffer), from_fd)) > 0)
-    if ((i = fwrite(buffer, j, 1, to_fd)) != 1)
-      return FALSE;
+  if ((j = read(src, buffer, sizeof(buffer))) > 0)
+    for (s = buffer; (i = write(dst, s, j)) != j; s += i, j -= i)
+      if (i < 0)
+       return FALSE;
+  else if (j < 0)
+    return FALSE;
 while (j > 0);
 return TRUE;
 }
@@ -90,21 +95,26 @@ copy_spool_files(transport_instance * tb, address_item * addr,
   int sdfd, int ddfd, BOOL link_file, int srcfd)
 {
 BOOL is_hdr_file = srcfd < 0;
-uschar * suffix = srcfd < 0 ? US"H" : US"D";
+const uschar * suffix = srcfd < 0 ? US"H" : US"D";
 int dstfd;
-FILE * dst_file, * src_file;
-uschar * filename = string_sprintf("%s-%s", message_id, suffix);
-uschar * srcpath = spool_fname(US"input", message_subdir, message_id, suffix);
-uschar * dstpath = string_sprintf("%s/%s-%s",
+const uschar * filename = string_sprintf("%s-%s", message_id, suffix);
+const uschar * srcpath = spool_fname(US"input", message_subdir, message_id, suffix);
+const uschar * dstpath = string_sprintf("%s/%s-%s",
   ((queuefile_transport_options_block *) tb->options_block)->dirname,
   message_id, suffix);
+const uschar * s;
+const uschar * op;
 
 if (link_file)
   {
   DEBUG(D_transport) debug_printf("%s transport, linking %s => %s\n",
     tb->name, srcpath, dstpath);
 
-  return linkat(sdfd, CCS filename, ddfd, CCS filename, 0) >= 0;
+  if (linkat(sdfd, CCS filename, ddfd, CCS filename, 0) >= 0)
+    return TRUE;
+
+  op = US"linking";
+  s = dstpath;
   }
 
 /* use data copy */
@@ -112,63 +122,29 @@ if (link_file)
 DEBUG(D_transport) debug_printf("%s transport, copying %s => %s\n",
   tb->name, srcpath, dstpath);
 
-if ((dstfd =
-  openat(ddfd, CCS filename, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE)) < 0)
-  {
-  addr->transport_return = DEFER;
-  addr->basic_errno = errno;
-  addr->message = string_sprintf("%s transport opening file: %s "
-    "failed with error: %s", tb->name, dstpath, strerror(errno));
-  return FALSE;
-  }
-
-fchmod(dstfd, SPOOL_MODE);
-
-if (!(dst_file = fdopen(dstfd, "wb")))
-  {
-  addr->transport_return = DEFER;
-  addr->basic_errno = errno;
-  addr->message = string_sprintf("%s transport opening file fd: %s "
-    "failed with error: %s", tb->name, dstpath, strerror(errno));
-  (void) close(dstfd);
-  return FALSE;
-  }
-
-if (is_hdr_file)
-  if ((srcfd = openat(sdfd, CCS filename, O_RDONLY)) < 0)
-    {
-    addr->basic_errno = errno;
-    addr->message = string_sprintf("%s transport opening file: %s "
-      "failed with error: %s", tb->name, srcpath, strerror(errno));
-    goto bad;
-    }
-
-if (!(src_file = fdopen(srcfd, "rb")))
-  {
-  addr->basic_errno = errno;
-  addr->message = string_sprintf("%s transport opening file fd: "
-    "%s failed with error: %s", tb->name, srcpath, strerror(errno));
-  if (is_hdr_file) (void) close(srcfd);
-  goto bad;
-  }
-
-if (!copy_spool_file(dst_file, src_file))
-  {
-  addr->message = string_sprintf("%s transport creating file: "
-    "%s failed with error: %s", tb->name, dstpath, strerror(errno));
-  if (is_hdr_file) (void) fclose(src_file);
-  goto bad;
-  }
-
-if (is_hdr_file) (void) fclose(src_file);
-(void) fclose(dst_file);
-
-return TRUE;
-
-bad:
-  addr->transport_return = DEFER;
-  (void) fclose(dst_file);
-  return FALSE;
+if (  (s = dstpath,
+       (dstfd = openat(ddfd, CCS filename, O_RDWR|O_CREAT|O_EXCL, SPOOL_MODE))
+       < 0
+      )
+   ||    is_hdr_file
+      && (s = srcpath, (srcfd = openat(sdfd, CCS filename, O_RDONLY)) < 0)
+   )
+  op = US"opening";
+
+else
+  if (s = dstpath, fchmod(dstfd, SPOOL_MODE) != 0)
+    op = US"setting perms on";
+  else
+    if (!copy_spool_file(dstfd, srcfd))
+      op = US"creating";
+    else
+      return TRUE;
+
+addr->basic_errno = errno;
+addr->message = string_sprintf("%s transport %s file: %s failed with error: %s",
+  tb->name, op, s, strerror(errno));
+addr->transport_return = DEFER;
+return FALSE;
 }
 
 /*************************************************