Re: replace use of mkstemp in makeinfo.m with tmpnam/fopen ?

[Benjamin Lindner]

John W. Eaton wrote:
> On 30-Jan-2009, Benjamin Lindner wrote:
>
> | with the current develeopment sources, makeinfo.m utilizes the function 
> | mkstemp to create its temporary texinfo file.
> | However, mkstemp is not available on mingw32, so its call returns a file 
> | descriptor of -1 and subsequently, makeinfo.m fails. This means that 
> | help() and pring_usage() also fail.
> | 
> | Is is acceptable to replace the call to mkstemp() with a call to 
> | tmpnam() and fopen() instead?
> | This way, makeinfo.m does not use functions that may not be available on 
> | some OSes.
> | 
> | I have attached a proposal changeset.
> | This yields working makeinfo.m and thus also help() and print_usage() 
> | for a mingw32 platform
>
> Would it maybe be better to use a similar method in mkstemp itself so
> that it can work on systems that don't have the mkstemp library
> function?  I think we would need a disclaimer in the documentation
> saying that on systems that don't have the mkstemp library function,
> Octave's mkstemp does not have the same security properties that
> mkstemp does.

After some research, I found that mingw gcc provides a mkstemps() 
function in libiberty.a which can be (and is) used as replacement for 
mkstemp().
This is also recommended practice according to gnu coding standards.

I added an autoconf check for the mingw platform and adapted the code in 
file-io.cc - see the attached changeset.

Is this an acceptable compromise?

I'd like to see this fixed for the next release, since it renders the 
"help" command useless otherwise.

benjamin

# HG changeset patch
# User Benjamin Lindner <address@hidden>
# Date 1234874540 -3600
# Node ID ae49c0b3b80cc89024dca9cda933b4fc0ed7fd6a
# Parent  57f74fbd3e767d2562a31a77dd87da28506aacd3
use mkstemps as replacement for mkstemp on mingw32

diff -r 57f74fbd3e76 -r ae49c0b3b80c ChangeLog
--- a/ChangeLog Tue Feb 17 08:24:05 2009 +0100
+++ b/ChangeLog Tue Feb 17 13:42:20 2009 +0100
@@ -1,3 +1,8 @@
+2009-02-17  Benjamin Lindner  <address@hidden>
+
+       * configure.in: Check for mkstemps() on mingw platform
+       (HAVE_MKSTEMPS): Define if mkstsmps() is avilable in libiberty
+
 2009-02-16  Jaroslav Hajek  <address@hidden>
 
        * NEWS: Yet more updates.
diff -r 57f74fbd3e76 -r ae49c0b3b80c configure.in
--- a/configure.in      Tue Feb 17 08:24:05 2009 +0100
+++ b/configure.in      Tue Feb 17 13:42:20 2009 +0100
@@ -1585,6 +1585,28 @@
   _chmod _snprintf x_utime _utime32)
 
 case "$canonical_host_type" in
+  *-*-mingw*)
+    ## MinGW does not provide a mkstemp function. However, it provides the 
mkstemps
+    ## function in libiberty, which can be used as replacement
+    AC_MSG_CHECKING(for mkstemps in libiberty)
+    save_LIBS="$LIBS"
+    LIBS="-liberty $LIBS"
+    AC_LINK_IFELSE([
+      AC_LANG_PROGRAM([[int mkstemps (char *pattern, int suffix_len);]], 
+       [[mkstemps("XXXXXX",0);]]
+    )],
+    [AC_MSG_RESULT(yes)
+     HAVE_MKSTEMPS=yes
+     AC_DEFINE(HAVE_MKSTEMPS, 1, [Define if mkstemps is available in 
libiberty.])
+    ],
+    [AC_MSG_RESULT(no)
+     HAVE_MKSTEMPS=no
+     LIBS="$save_LIBS"
+    ])
+   ;;
+esac
+
+case "$canonical_host_type" in
   *-*-msdosmsvc)
     ## The %T format specifier for strftime is reportedly broken,
     ## so use our version.  We could use an actual configure test
diff -r 57f74fbd3e76 -r ae49c0b3b80c src/ChangeLog
--- a/src/ChangeLog     Tue Feb 17 08:24:05 2009 +0100
+++ b/src/ChangeLog     Tue Feb 17 13:42:20 2009 +0100
@@ -1,3 +1,8 @@
+2009-02-17  Benjamin Lindner  <address@hidden>
+
+       * file-io.cc: (HAVE_MKSTEMPS): Use mkstemps() in Fmktemps if available 
as
+       substitute for mkstemp()
+
 2009-02-16  John W. Eaton  <address@hidden>
 
        * genprops.awk (emit_source): Don't use + to concatenate strings.
diff -r 57f74fbd3e76 -r ae49c0b3b80c src/file-io.cc
--- a/src/file-io.cc    Tue Feb 17 08:24:05 2009 +0100
+++ b/src/file-io.cc    Tue Feb 17 13:42:20 2009 +0100
@@ -1901,6 +1901,11 @@
   return retval;
 }
 
+#if defined (HAVE_MKSTEMPS)
+// Prototype for mkstemps in libiberty
+extern "C" int mkstemps (char *pattern, int suffix_len);
+#endif
+
 DEFUN (mkstemp, args, ,
   "-*- texinfo -*-\n\
 @deftypefn {Built-in Function} address@hidden, @var{name}, @var{msg}] =} 
mkstemp (@var{template}, @var{delete})\n\
@@ -1929,7 +1934,7 @@
   retval(1) = std::string ();
   retval(0) = -1;
 
-#if defined (HAVE_MKSTEMP)
+#if defined (HAVE_MKSTEMP) || defined (HAVE_MKSTEMPS)
 
   int nargin = args.length ();
 
@@ -1942,7 +1947,11 @@
          OCTAVE_LOCAL_BUFFER (char, tmp, tmpl8.size () + 1);
          strcpy (tmp, tmpl8.c_str ());
 
+#if defined (HAVE_MKSTEMP)
          int fd = mkstemp (tmp);
+#else
+         int fd = mkstemps (tmp, 0);
+#endif
 
          if (fd < 0)
            {