Re: [bug-diffutils] Diffutils 3.2 v. VMS

[Paul Eggert]

Thanks for the heads-up.  Some comments:

On 10/08/2012 09:46 PM, Steven M. Schweda wrote:

>    There are still some minor compiler complaints caused by time_t being
> unsigned around here:

That's OK.  The code's correct, so the compiler diagnostics are mistaken,
so we can ignore them.  Perhaps you can use compiler flags to shut
them off.

> %CC-I-LONGDOUBLENYI, In this declaration, type long double has the same
>  representation as type double on this platform.

Again, that's OK.  The code's fine, and works correctly when long double
is the same as double, so the warnings are not helpful and can be ignored.
Some POSIXish systems are similar.  They have unsigned time_t and
sometimes their compilers warn.  We ignore the warnings there too.

So this change:

-                     if (value[i] == -1)
+                     if (value[i] == (uintmax_t)-1)

should not be needed: value[i] is of type uintmax_t, so the
comparison should be done correctly without the cast.

As for the other changes, I'd rather not have "#ifdef __VMS"
in the mainline code.  The mainline code should be as close to
GNU- or POSIX-compatible as possible.  VMS-specific stuff should
be in a separate area, so that maintainers of the regular code
don't need to worry about it.  For example, instead of this:

+#ifdef __VMS
+# define OPEN_ARG , "ctx = stm"
+#else /* def __VMS */
+# define OPEN_ARG
+#endif /* def __VMS [else] */
+
 #if defined LC_MESSAGES && ENABLE_NLS
 # define hard_locale_LC_MESSAGES hard_locale (LC_MESSAGES)
 #else
@@ -296,7 +302,7 @@
            xfreopen (NULL, "rb", stdin);
        }
       else
-       file_desc[f1] = open (file[f1], O_RDONLY | O_BINARY, 0);
+       file_desc[f1] = open (file[f1], O_RDONLY | O_BINARY, 0 OPEN_ARG);

I'd rather that we leave this code alone, and have the wrapper
for 'open' (in the lib subdirectory) deal with OPEN_ARG.  And
instead of this:

+#ifdef __VMS
+  /* No "/" required (or desired) on VMS. */
+  sprintf (tmpl, "%.*s%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx);
+#else /* def __VMS */
   sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx);
+#endif /* def __VMS [else] */

I'd rather have something like this:

#ifndef DIRECTORY_SEPARATOR_STRING
# define DIRECTORY_SEPARATOR_STRING "/"
#endif

   sprintf (tmpl, "%.*s" DIRECTORY_SEPARATOR_STRING "%.*sXXXXXX", (int) dlen, 
dir, (int) plen, pfx);

where some other file (dirname.h, perhaps) defines DIRECTORY_SEPARATOR_STRING
for VMS.