Re: symlink.c for Windows

[Eli Zaretskii]

> From: Dmitry Selyutin <address@hidden>
> Date: Mon, 9 Sep 2013 02:27:13 +0400
> 
> Sorry for the long absence, there are lot of other things to be done. Still
> I'm trying to participate, though, as I can. Recently I've found that there
> is no symlink function for Windows, though Windows has CreateSymbolicLink
> function. I've tried to implement my own. I don't know C or C++ really
> well, but fix seemed to be trivial enough. MinGW doesn't support symlinks,
> though reports success.

Caveat: I'm not a gnulib developer or maintainer, so what's below in
no way reflects the opinions of the project about your contribution.

That said, please allow me a few comments.

First, I think having 'symlink' is just a small part of support for
symbolic links.  Functions such as 'lstat' (a real one, not an alias
of 'stat'), 'stat' that reports about the target of the symlink, and
'readlink' are also needed.  Unlike with 'symlink', where the Windows
implementation is simple, 'readlink' (and to some degree 'stat') are
not, because there's no easy-to-use API on Windows that allows to find
the target of a symlink.

In addition, some other library functions, like 'access' and 'chmod',
need to resolve symlinks internally, because the Windows APIs used by
these functions report the attributes of the symlink, not of its
target.

IMO, having just 'symlink' without all the rest will provide a very
incomplete support for symbolic links.

A couple of comments to your code:

> +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
> +
> +#  include <windows.h>
> +
> +#  if defined __MINGW32__
> +/* MinGW does not support symlinks. */

I don't understand: if the code you wrote is not for MinGW, then for
what Windows development environment is it, and how does that
environment support symlinks, if it doesn't have the 'symlink'
function?

> +  /* Reject trailing slashes on non-directories. */
> +  if ((len1 && (file1[len1 - 1] == '/' || file1[len1 - 1] == '\\'))
> +      || (len2 && (file2[len2 - 1] == '/' || file2[len2 - 1] == '\\')))

The test of the trailing backslash will not do what you want in a
Windows locale that uses DBCS character sets, because there the second
byte of a two-byte character can be a backslash, even though the
character is some CJK character, not a directory separator.  The
result will be that the function will reject a perfectly valid file
name.

> +      if (stat (file1, &st) == 0 && S_ISDIR (st.st_mode))
> +        errno = EPERM;

This doesn't allow creation of symbolic links to non-existent
directories, which is a subtlety Posix programs might not expect
(because Posix platforms don't distinguish between symlinks to
directories and non-directories).

Thanks again for working on this.