Re: [Chicken-hackers] [PATCH] Fix #819 by marking file-mkstemp as unimplemented on Windows

[Mario Domenech Goulart]

Hi Michele,

On Sun, 3 Aug 2014 01:45:22 +0200 Michele La Monaca <address@hidden> wrote:

> On Sat, Aug 2, 2014 at 3:10 PM, Peter Bex <address@hidden> wrote:
>>
>> This is an old bug and I think it's a good idea to remove file-mkstemp
>> from the Windows version of the posix unit.  It's been broken from the
>> beginning, and nobody noticed.  Maybe nobody is using it (I hope!), but
>> it could also be that people are using it and expecting it to return
>> sensible values.
>>
>> In any case, I opted for removing it because this is just extremely
>> tricky to get right.  We're better off removing it than keeping the
>> broken version, so if you prefer to keep the procedure and want to
>> do it properly, please write a proper patch.
>
> I think this function is too important to be dismissed so easily.
> Attached an implementation which gets rid of the limited mktemp
> function. Actually a quick untested hack (I don't have access to a
> mingw setup right now) but I hope a step in the right direction.

...
+        (let ((prefix (##sys#substring tmpl 0 (- (string-length tmpl) x-len))))
+          (let loop ((count 1))
+            (let ((suffix (let suffix-loop ((index 0))
+                            (if (>= index x-len)
+                              ""
+                              (string-append
+                                (string (string-ref diz (random diz-len)))
+                                (suffix-loop (+ index 1)))))))
+              (let ((f (tmpl-full-path (string-append prefix suffix))))
+                (if (file-exists? f)

If I understand correctly, at this point the code is subject to race
conditions, which is one of the things mkstemp prevents.

I think it's worth mentioning that both create-temporary-file and
create-temporary-directory from the files unit are subject to race
conditions too.  create-temporary-directory is remarkably more fragile,
since it uses directory-exists?, which would make
create-temporary-directory raise an error in case a non-directory (a
regular file, symlink etc.) exists with the same name as the attempted
name for the temporary directory.

+                  (if (< count max-attempts)
+                    (loop (+ count 1))
+                    (##sys#error 'mkstemp "max attempts reached"))
+                  f)))))))
...


Best wishes.
Mario
-- 
http://parenteses.org/mario