This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: fix build error on MinGW (HAVE_READLINK) undefined
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Pedro Alves <palves at redhat dot com>
- Cc: asmwarrior at gmail dot com, gdb-patches at sourceware dot org
- Date: Fri, 27 Jan 2012 13:59:53 +0200
- Subject: Re: fix build error on MinGW (HAVE_READLINK) undefined
- References: <4F2206DE.20907@gmail.com> <83k44dzejs.fsf@gnu.org> <4F22760A.2020309@redhat.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> 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. 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"? 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?
> >> 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?