On Tue, Nov 12, 2024 at 09:35:24PM +0000, Gavin Smith wrote:
> I can't see how it can be that complicated to call fopen/fread/fwrite
> ourselves to copy the file.  These are all completely standard library
> functions.  I can have a go at implementing this in the next couple of
> days.

My work on this is at the bottom of this message.

I have tested the code and it passes the tests with TEXINFO_XS_CONVERT=1,
although I expect some of the error handling code is hard to test properly.

I looked at the implementation in gnulib/lib/copy-file.c with the
thought of copying some of the code, but it mostly used the low-level
open/read/write API instead of fopen/fread/fwrite which is probably
more portable.

I was unsure on how the file names should be printed in error messages.

The current version has this code:

     xasprintf (&from, "%s/%s", jssrcdir, js_files[i]);
     xasprintf (&to, "%s/%s", encoded_jsdir, js_files[i]);
     status = copy_file_to (from, to);
     if (status != 0)
       {
         char *to_file_name;
         char *from_file_name;
         xasprintf (&to_file_name, "%s/%s", jsdir, js_files[i]);
         xasprintf (&from_file_name, "%s/%s", jsdir, js_files[i]); 

Different strings are used for opening the files as for printing
the error messages.  This would make sense if the file names should
be encoded for writing the file (according to the value of the
OUTPUT_FILE_NAME_ENCODING customization variable), but not for
passing to the error message function ('message_list_document_error').

However, the assignment to 'from_file_name' seems wrong, as this should
be be based on 'jssrcdir' not 'jsdir'.  The source and destination file
names shouldn't both use the same directory as the source is likely in
some installed location, while the destination with the HTML output files.

Should this be the following?

         xasprintf (&from_file_name, "%s/%s", jssrcdir, js_files[i]);
         xasprintf (&to_file_name, "%s/%s", jsdir, js_files[i]);

With this, 'from_file_name' is the same as 'from'.

For MS-Windows I checked if O_BINARY or _O_BINARY was set, as is done in
the top-level system.h include (although I didn't include system.h).

I reused most of the error messages from the existing code, although
didn't find a use for some of them (for GL_COPY_ERR_AFTER_READ and the
ACL errors).


diff --git a/tp/Texinfo/XS/convert/convert_html.c 
b/tp/Texinfo/XS/convert/convert_html.c
index 2aec97aa3b..d4ef4e3424 100644
--- a/tp/Texinfo/XS/convert/convert_html.c
+++ b/tp/Texinfo/XS/convert/convert_html.c
@@ -20,8 +20,6 @@
 #include <stdio.h>
 #include <errno.h>
 
-#include "copy-file.h"
-
 #include "html_conversion_data.h"
 #include "text.h"
 #include "element_types.h"
@@ -2392,6 +2390,95 @@ do_jslicenses_file (CONVERTER *self)
   free (formatted_jlicenses);
 }
 
+/* Set FOPEN_RBIN and ROPEN_WBIN. */
+#ifndef O_BINARY
+# ifdef _O_BINARY
+#  define O_BINARY _O_BINARY
+# else
+  #  define O_BINARY 0
+  # endif
+#endif /* O_BINARY */
+
+#if O_BINARY /* MS-Windows */
+# define FOPEN_RBIN     "rb"
+# define FOPEN_WBIN     "wb"
+#else
+# define FOPEN_RBIN     "r"
+# define FOPEN_WBIN     "w"
+#endif
+
+static void
+copy_file_to (CONVERTER *self, const char *from, const char *to)
+{
+  FILE *src = 0, *dest = 0;
+
+  const char *from_file_name = from, *to_file_name = to;
+
+  src = fopen (from, FOPEN_RBIN);
+  if (!src)
+    {
+      message_list_document_error (&self->error_messages,
+          self->conf, 0,
+          "error while opening %s for reading: %s",
+          from_file_name, strerror (errno));
+      return;
+    }
+
+  dest = fopen (to, FOPEN_WBIN);
+  if (!dest)
+    {
+      message_list_document_error (&self->error_messages,
+          self->conf, 0,
+          "cannot open %s for writing: %s",
+          to_file_name, strerror (errno));
+      fclose (src);
+      return;
+    }
+
+#define bufsize 512
+  char buf[bufsize];
+  size_t nread, nwritten;
+  do
+    {
+      nread = fread (buf, sizeof(char), sizeof(buf), src);
+
+      nwritten = fwrite (buf, sizeof(char), nread, dest);
+      if (nwritten != nread)
+        {
+          message_list_document_error (&self->error_messages,
+              self->conf, 0,
+              "error writing %s: %s",
+              to_file_name, strerror (errno));
+          fclose (src);
+          fclose (dest);
+          return;
+        }
+    }
+  while (nread == bufsize);
+#undef bufsize
+
+  if (ferror (src))
+    {
+      message_list_document_error (&self->error_messages,
+          self->conf, 0,
+          "error reading %s: %s",
+          from_file_name, strerror (errno));
+      fclose (src);
+      fclose (dest);
+      return;
+    }
+
+ fclose (src);
+ if (fclose (dest) != 0)
+   {
+      message_list_document_error (&self->error_messages,
+          self->conf, 0,
+          "error closing %s: %s",
+          to_file_name, strerror (errno));
+      return;
+   }
+}
+
 static const char *js_files[4] = {"info.js", "modernizr.js", "info.css", 0};
 
 void
@@ -2448,81 +2535,12 @@ html_do_js_files (CONVERTER *self)
                 {
                   char *from;
                   char *to;
-                  int status;
 
                   xasprintf (&from, "%s/%s", jssrcdir, js_files[i]);
                   xasprintf (&to, "%s/%s", encoded_jsdir, js_files[i]);
-                  status = copy_file_to (from, to);
-
-                  if (status != 0)
-                    {
-                      char *to_file_name;
-                      char *from_file_name;
 
-                      xasprintf (&to_file_name, "%s/%s", jsdir, js_files[i]);
-                      xasprintf (&from_file_name, "%s/%s", jsdir, js_files[i]);
+                  copy_file_to (self, from, to);
 
-                      switch (status)
-                        {
-                          case GL_COPY_ERR_OPEN_READ:
-                            message_list_document_error (&self->error_messages,
-                                self->conf, 0,
-                                "error while opening %s for reading: %s",
-                                from_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_OPEN_BACKUP_WRITE:
-                            message_list_document_error (&self->error_messages,
-                                self->conf, 0,
-                                "cannot open %s for writing: %s",
-                                to_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_READ:
-                            message_list_document_error (&self->error_messages,
-                                self->conf, 0,
-                                "error reading %s: %s",
-                                from_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_WRITE:
-                            message_list_document_error (&self->error_messages,
-                                self->conf, 0,
-                                "error writing %s: %s",
-                                to_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_AFTER_READ:
-                            message_list_document_error (&self->error_messages,
-                                self->conf, 0,
-                                "error after reading %s: %s",
-                                from_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_GET_ACL:
-                            message_list_document_warn (&self->error_messages,
-                                self->conf, 0,
-                                "%s: %s",
-                                from_file_name, strerror (errno));
-                            break;
-
-                          case GL_COPY_ERR_SET_ACL:
-                            message_list_document_warn (&self->error_messages,
-                                self->conf, 0,
-                                "preserving permissions for %s: %s",
-                                to_file_name, strerror (errno));
-                            break;
-
-                          default:
-                            message_list_document_warn (&self->error_messages,
-                                self->conf, 0,
-                                "unexpected error on copying %s into %s",
-                                from_file_name, to_file_name);
-                            break;
-                        }
-                      free (to_file_name);
-                      free (from_file_name);
-                    }
                   free (to);
                   free (from);
                 }


Reply via email to