Re: symlink.c for Windows

[Dmitry Selyutin]

Hello, Eli!

First of all, thanks for your explanations! As I've said, I'm not so skilled developer in C or C++.

As for compilers and toolchains, I'm pretty sure MinGW is not the only supported platform, since we have a lot of checks for things like MSVC inside gnulib files. gnulib is compatibility library which aims to support set of functions and types for cross-platform develop, so yes, I don't think MinGW is the only choice.

As for trailing slashes, the code was taken from gnulib's link.c. If it needs a fix, it would be a good idea to fix it in all places. Probably it worths adding support for DBCS locale, though it may be hard to add it everywhere. Probably you could do it, since I'm not so good in Windows locales? Thanks!

As for other library functions, I'm going to work deeper on this part when I'll have time. It is possible to write wrappers around Windows functions. I know that this may be a hard thing to do, but it is possible to do. I'll try to add such functionality as soon as I can. Since system _supports_ symlinks, imho, we shall try to add support for it if we can do it.

Thanks for your kind explanations, remarks and advice. It's always a great pleasure and honor for me to learn from experienced professionals, and I'm really glad that here in gnulib I sometimes have such an opportunity. Thank you again! If there are some things to be said, I'm always open for discussion!

2013/9/9 Eli Zaretskii <address@hidden>

> 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.

--

With best regards,

Dmitry Selyutin

E-mail: address@hidden

Phone: +7(985)334-07-70