Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Pedro Alves <palves@redhat.com>
Cc: asmwarrior@gmail.com, gdb-patches@sourceware.org
Subject: Re: fix build error on MinGW (HAVE_READLINK) undefined
Date: Fri, 27 Jan 2012 12:22:00 -0000	[thread overview]
Message-ID: <83ehulz77q.fsf@gnu.org> (raw)
In-Reply-To: <4F22760A.2020309@redhat.com>

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


  reply	other threads:[~2012-01-27 12:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-27  3:20 asmwarrior
2012-01-27  9:41 ` Eli Zaretskii
2012-01-27 10:28   ` Pedro Alves
2012-01-27 12:22     ` Eli Zaretskii [this message]
2012-01-27 12:36       ` Pedro Alves
2012-01-27 12:47         ` Eli Zaretskii
2012-02-03 11:47     ` [RFA/gdbserver] Provide dummy readlink on systems where routine is not available Joel Brobecker
2012-02-03 11:56       ` Joel Brobecker
2012-02-03 12:31         ` Joel Brobecker
2012-02-03 13:11           ` Eli Zaretskii
2012-02-03 13:26             ` Joel Brobecker
2012-02-09 16:32     ` [RFA/commit] gdbserver: return ENOSYS if readlink not supported Joel Brobecker
2012-02-09 16:41       ` Pedro Alves
2012-02-09 17:32         ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83ehulz77q.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=asmwarrior@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox