[patch] Fix g10/exec.c (make_tempdir) [_WIN32] not properly handling GetTempPath

[Israel G. Lugo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
 
David Shaw wrote:
> I think the proposed fix is a bit of overkill.  I'd rather do
> something simpler, like (not tested yet):
That is still enforcing a hardcoded limit, though, of MAX_PATH
characters (which is defined to 260 in the 2003 SDK). If we can behave
elegantly and support arbitrarily long paths with ease, why would we
want to impose an unnecessary artificial limit on ourselves?

If the user's temporary directory happens to be longer than 260 bytes
(think localized Windows versions, or large usernames, or multibyte
locales or whatever), the alternative solution you quoted will reject
it (and fall back to the non-portable c:\windows\temp thing), when it
wasn't really necessary to do so.

It kind of reminds me of the whole "2 digits should be more than
enough to represent the year" thing - or, in this case, "260 bytes
should be more than enough for everyone" ;) Why not aim to be scalable
and flexible now, if we can easily do it without introducing any other
problems? (in fact it's already done in the solution I proposed)

I believe if it is possible to improve on something, even if only a
bit, then it should be done, unless it is counterproductive to do so.
After all, a very important part of the Free Software spirit (or Open
Source, however one might prefer to think of it) is to have an open
development model, with people from the community trying to work
together and with those people from outside the project who are
motivated enough to contribute and want to help the project be the
best it can be.

> There is no need to get the Windows directory specially, as
> GetTempPath will return the Windows directory as a last resort.  It
>  tries $TMP, $TEMP, $USERPROFILE, and finally the Windows
> directory.
That is true; I suppose I was only trying to make the program a bit
more resilient. We don't really know for what reasons GetTempPath
might fail (some internal error, etc). It could fail for a reason
other than not being able to detect the Windows directory; in such a
case, GPG might still be able to get the Windows directory, by using
GetWindowsDirectory (if that fails too then we could always fall back
to the hardcoded thing).

> I must admit I'm not terribly concerned that some version of
> Windows somewhere can't handle c:\windows\temp.  If we get that
> far, it means GetTempPath failed in some manner, and if GetTempPath
> fails with MAX_PATH+1, then the system is sufficiently broken that
> we likely don't *want* to continue executing. ;)
My own Windows installation is a good example which would not work
with c:\windows\temp ;) I have it on F:\winnt.

Any NT or 2000 Windows will be on \winnt by default, for example.
Localized versions in some languages will probably install in a
different path as well (or at least the \temp part, it might be called
something else). And of course, one can always customize his
installation path upon installing the OS. So, I don't think hardcoding
is the best solution, at least not the way it was done in the
alternative solution you posted, since the hardcoded case can be
entered for more reasons than just a broken system (temp path longer
than the hardcoded limit of MAX_PATH, or some internal error inside
GetTempPath, for example). Falling back to c:\windows\temp as a last
resort if GetLastTemp errored out might make sense (although it could
potentially still be a recoverable error), but doing so because the
path is longer than some artificially imposed limit would seem a bit odd.

Ideally, I still think there should be an explicit check using
GetWindowsDirectory before giving up, but I suppose that might be
overkill if we're at least sure that we do handle valid temp paths
properly, regardless of their length.

Now, I dislike Windows as much as anyone else. I would be a happy man
if everyone used some form of free operating system such as GNU, *BSD
or whatever. However, that is besides the point. If GnuPG is
officially supposed to work on Windows, then it should do so to the
best of its abilities, like it does on the other OSes for which it is
developed. If GnuPG breaks on Windows because the user's temporary
path was longer than some imposed limit, people are not going to blame
Windows, they're going to blame GnuPG.

I understand it may perhaps be difficult for some of the GPG
developers to be motivated to develop for the Windows platform, or
even to test the program on it. I myself use GNU/Linux for most of my
computing tasks, but I do regrettably have to use Windows as well.
Should there be a need, I would be happy to help with any development
work, as I am comfortable with both platforms and could run and test
the program on either.

In any event, I would submit that the solution I proposed initially,
with the handling of arbitrarily long paths, would be a good
compromise between not being flexible enough (hardcoded limits, etc)
and being "too" flexible (checking the Windows dir after GetTempPath
failed, and so on). It may add 4 or 5 lines of code, but there really
isn't much getting around the GetTempPath interface: there is simply
no guarantee on what the temp path length will be. One really has to
poll and fetch (or at least make an initial guess hoping that it will
be long enough, but a loop is inevitable to cover the case when the
initial guess is not long enough).

Regards,
Israel

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
 
iD8DBQFEdNUfgn9msFnMGkcRAnMWAKCKV+fDpnAsCZIFFfBfydyIFEhm8wCfebTd
rQFmOD3SaTQKdK3vq74dGxE=
=g60v
-----END PGP SIGNATURE-----