Re: fix build error on MinGW (HAVE_READLINK) undefined

[Pedro Alves <palves@redhat.com>]

On 01/27/2012 11:59 AM, Eli Zaretskii wrote:
>> Date: Fri, 27 Jan 2012 10:01:46 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: asmwarrior <asmwarrior@gmail.com>, gdb-patches@sourceware.org
>>
>>> I think you need to set errno to EINVAL in the #else branch, because
>>> the meaning of that error on systems that do have readlink is "the
>>> named file is not a symbolic link".
>>>
>>> On second thought, perhaps a better way would be to define a readlink
>>> for MinGW that always sets errno to EINVAL and returns -1.  Then the
>>> ugly #ifdef can go away.
>>
>> The corresponding native side returns ENOSYS/FILEIO_ENOSYS, indicating
>> the function is not supported by the implementation.  GDBserver should do
>> the same.
> 
> But isn't ENOSYS sub-optimal in this case?  Systems that don't have
> readlink don't have symlinks, either.  

Not necessarily true.  Might have symlinks but not a defined MAXPATHLEN,
for example (I guess the Hurd may fall on that basked).  Might have
symlinks, but need some other mechanism or system call to read them.  The
error is really "the `readlink' operation is not supported", so ENOSYS is
the best fit.

> So any file there is trivially,
> by definition, not a symlink.  Why not tell that to the caller, and
> have the rest of the code work "normally"?

Because it isn't what happened.  You'd still confuse the client side.
E.g., if the file does not exist, we should return ENOENT instead.

> By contrast, setting errno
> to ENOSYS will most likely be processed as an exceptional situation,
> like failing the command that invoked readlink.  How is that TRT?

The code on the client that invoked readlink should cope.  There's no
such thing currently, but if there was a generic command that invoked a
readlink on a Windows target, and it got ENOSYS, it would then say
"operation not supported" if it chose, or fallback to some other means.
Returning "not a symlink" could confuse the client.

> 
>>>> static char *
>>>> inf_child_fileio_readlink (const char *filename, int *target_errno)
>>>> {
>>>>    /* We support readlink only on systems that also provide a compile-time
>>>>       maximum path length (MAXPATHLEN), at least for now.  */
>>>> #if defined (HAVE_READLINK) && defined (MAXPATHLEN)
>>>>    char buf[MAXPATHLEN];
>>>>    int len;
>>>>    char *ret;
>>>>
>>>>    len = readlink (filename, buf, sizeof buf);
>>>
>>> Here too.
>>
>> The #else branch just below reads:
>>
>> #else
>>   *target_errno = FILEIO_ENOSYS;
>>   return NULL;
>> #endif
> 
> Which is wrong, don't you think?

I don't.

-- 
Pedro Alves