Re: [Patch, libfortran] Fix handling of temporary files

[Tobias Burnus <tobias dot burnus at physik dot fu-berlin dot de>]

Dear Janne,

sorry for the really slow review.

Janne Blomqvist wrote:
> the attached patch implements some fixes for handling STATUS="SCRATCH" files.
...
> Currently, we check [...] but not the POSIX and GNU standard TMPDIR [...]
> If the program is privileged, we shouldn't trust path style environment variables.

Good chatch!


+On the MingW target, the directory returned by the @code{GetTempPath}
+#define P_tmpdir _P_tmpdir  /* MingW */

It's spelt MinGW ("Minimalist GNU for Windows").


 * GFORTRAN_STDERR_UNIT:: Unit number for standard error
-* GFORTRAN_TMPDIR:: Directory for scratch files
+* TMPDIR:: Directory for scratch files
 * GFORTRAN_UNBUFFERED_ALL:: Do not buffer I/O for all units.

I would re-order the list and place TMPDIR either at the top or at
the bottom. Given that it is a more useful environment variable,
placing it at the top might make sense.
(You should then also move the actual text.)


+ /* Check for special case that tempdir contains slash
      or backslash at end.  */

(second line looks wrongly indented.)

Additionally, I would add a couple of articles:
  "Check for the special case that tempdir contains a slash
   or backslash at the end"
or, better,
  "Check for the special case that tempdir ends with a slash
   or backslash"


+tempfile_open (const char * tempdir, char ** fname)

GNU style: No spaces after "*".


+}
+
+/* tempfile()-- Generate a temporary filename for a scratch file and

Second empty line missing.


+  if (!tempdir)
+    return -1;
...
+  size_t tempdirlen = strlen (tempdir);

I am surprised that it compiles. I thought GCC is compiled with C89
settings such that variables have to be declared before the the
expression statements start. Did you check whether it compiles with
any warnings?

Otherwise, the patch looks fine.

Thanks for the patch!

Tobias

PS: Side note, I find the document produced by the ISO/IEC Project 22.24772:
"Guidance for Avoiding Vulnerabilities through Language Selection"
useful; while it is rather lengthy and not that readably written, it still
contains some helpful bits. However, it does not really address the
environment variable issue which you mentioned (ignoring 6.47.1 and
3.1.2.12 which are only loosely related).
Cf. http://gcc.gnu.org/wiki/GFortranStandards#ISO.2BAC8-IEC_Project_22.24772:_Guidance_for_Avoiding_Vulnerabilities_through_Language_Selection_and_Use