Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* fix build error on MinGW (HAVE_READLINK) undefined
@ 2012-01-27  3:20 asmwarrior
  2012-01-27  9:41 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: asmwarrior @ 2012-01-27  3:20 UTC (permalink / raw)
  To: gdb-patches

Not sure the patch is correct, but it do build OK now.

  gdb/gdbserver/hostio.c |    5 +++++
  1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index 34e4fa8..7575fb1 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -459,6 +459,7 @@ handle_unlink (char *own_buf)
  static void
  handle_readlink (char *own_buf, int *new_packet_len)
  {
+#if defined (HAVE_READLINK) && defined (PATH_MAX)
    char filename[PATH_MAX], linkname[PATH_MAX];
    char *p;
    int ret, bytes_sent;
@@ -485,6 +486,10 @@ handle_readlink (char *own_buf, int *new_packet_len)
       to return a partial response, but simply fail.  */
    if (bytes_sent < ret)
      sprintf (own_buf, "F-1,%x", FILEIO_ENAMETOOLONG);
+#else
+   hostio_error (own_buf);
+   return;
+#endif
  }
  
  /* Handle all the 'F' file transfer packets.  */



The other question is:

Here it use : PATH_MAX, but some other place, it use MAXPATHLEN
see: inf-child.c

/* Read value of symbolic link FILENAME on the target.  Return a
    null-terminated string allocated via xmalloc, or NULL if an error
    occurs (and set *TARGET_ERRNO).  */
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);
   if (len < 0)
     {
       *target_errno = inf_child_errno_to_fileio_error (errno);
       return NULL;
     }

   ret = xmalloc (len + 1);
   memcpy (ret, buf, len);
   ret[len] = '\0';
   return ret;
#else
   *target_errno = FILEIO_ENOSYS;
   return NULL;
#endif
}


asmwarrior
ollydbg from codeblocks' forum


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: fix build error on MinGW (HAVE_READLINK) undefined
  2012-01-27  3:20 fix build error on MinGW (HAVE_READLINK) undefined asmwarrior
@ 2012-01-27  9:41 ` Eli Zaretskii
  2012-01-27 10:28   ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2012-01-27  9:41 UTC (permalink / raw)
  To: asmwarrior; +Cc: gdb-patches

> Date: Fri, 27 Jan 2012 10:07:26 +0800
> From: asmwarrior <asmwarrior@gmail.com>
> 
> diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
> index 34e4fa8..7575fb1 100644
> --- a/gdb/gdbserver/hostio.c
> +++ b/gdb/gdbserver/hostio.c
> @@ -459,6 +459,7 @@ handle_unlink (char *own_buf)
>   static void
>   handle_readlink (char *own_buf, int *new_packet_len)
>   {
> +#if defined (HAVE_READLINK) && defined (PATH_MAX)
>     char filename[PATH_MAX], linkname[PATH_MAX];
>     char *p;
>     int ret, bytes_sent;
> @@ -485,6 +486,10 @@ handle_readlink (char *own_buf, int *new_packet_len)
>        to return a partial response, but simply fail.  */
>     if (bytes_sent < ret)
>       sprintf (own_buf, "F-1,%x", FILEIO_ENAMETOOLONG);
> +#else
> +   hostio_error (own_buf);
> +   return;
> +#endif

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.

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: fix build error on MinGW (HAVE_READLINK) undefined
  2012-01-27  9:41 ` Eli Zaretskii
@ 2012-01-27 10:28   ` Pedro Alves
  2012-01-27 12:22     ` Eli Zaretskii
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Pedro Alves @ 2012-01-27 10:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: asmwarrior, gdb-patches

On 01/27/2012 09:21 AM, Eli Zaretskii wrote:
>> Date: Fri, 27 Jan 2012 10:07:26 +0800
>> From: asmwarrior <asmwarrior@gmail.com>
>>
>> diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
>> index 34e4fa8..7575fb1 100644
>> --- a/gdb/gdbserver/hostio.c
>> +++ b/gdb/gdbserver/hostio.c
>> @@ -459,6 +459,7 @@ handle_unlink (char *own_buf)
>>   static void
>>   handle_readlink (char *own_buf, int *new_packet_len)
>>   {
>> +#if defined (HAVE_READLINK) && defined (PATH_MAX)

You need to teach config.h about HAVE_READLINK.  Add readlink to
AC_CHECK_FUNCS in gdbserver/configure.ac, and regenerate (autoconf/autoheader).


>>     char filename[PATH_MAX], linkname[PATH_MAX];
>>     char *p;
>>     int ret, bytes_sent;
>> @@ -485,6 +486,10 @@ handle_readlink (char *own_buf, int *new_packet_len)
>>        to return a partial response, but simply fail.  */
>>     if (bytes_sent < ret)
>>       sprintf (own_buf, "F-1,%x", FILEIO_ENAMETOOLONG);
>> +#else
>> +   hostio_error (own_buf);
>> +   return;
>> +#endif
> 
> 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.

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

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: fix build error on MinGW (HAVE_READLINK) undefined
  2012-01-27 10:28   ` Pedro Alves
@ 2012-01-27 12:22     ` Eli Zaretskii
  2012-01-27 12:36       ` Pedro Alves
  2012-02-03 11:47     ` [RFA/gdbserver] Provide dummy readlink on systems where routine is not available Joel Brobecker
  2012-02-09 16:32     ` [RFA/commit] gdbserver: return ENOSYS if readlink not supported Joel Brobecker
  2 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2012-01-27 12:22 UTC (permalink / raw)
  To: Pedro Alves; +Cc: asmwarrior, gdb-patches

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: fix build error on MinGW (HAVE_READLINK) undefined
  2012-01-27 12:22     ` Eli Zaretskii
@ 2012-01-27 12:36       ` Pedro Alves
  2012-01-27 12:47         ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2012-01-27 12:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: asmwarrior, gdb-patches

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: fix build error on MinGW (HAVE_READLINK) undefined
  2012-01-27 12:36       ` Pedro Alves
@ 2012-01-27 12:47         ` Eli Zaretskii
  0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2012-01-27 12:47 UTC (permalink / raw)
  To: Pedro Alves; +Cc: asmwarrior, gdb-patches

> Date: Fri, 27 Jan 2012 12:21:59 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: asmwarrior@gmail.com, gdb-patches@sourceware.org
> 
> > 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).

Could use PATH_MAX instead.  No problem here.

> E.g., if the file does not exist, we should return ENOENT instead.

We could check that as well, and return ENOENT.

> >>   *target_errno = FILEIO_ENOSYS;
> >>   return NULL;
> >> #endif
> > 
> > Which is wrong, don't you think?
> 
> I don't.

Too bad.  In my experience, saying "this is not a symlink" does TRT
more often than not.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFA/gdbserver] Provide dummy readlink on systems where routine is not available.
  2012-01-27 10:28   ` Pedro Alves
  2012-01-27 12:22     ` Eli Zaretskii
@ 2012-02-03 11:47     ` Joel Brobecker
  2012-02-03 11:56       ` Joel Brobecker
  2012-02-09 16:32     ` [RFA/commit] gdbserver: return ENOSYS if readlink not supported Joel Brobecker
  2 siblings, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2012-02-03 11:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker

Hello,

Re: http://www.sourceware.org/ml/gdb-patches/2012-01/msg00931.html

This is an attempt to fix the gdbserver build failure on Windows (MinGW)
hosts due to a the missing `readlink' function.

This patch provides a dummy readlink for systems like Windows which
do not provide one.  The dummy version returns -1 and sets errno
to ENOSYS (function not implemented).

gdb/ChangeLog:

        * common/gdb_readlink.h, common/gdb_readlink.c: New files.

gdb/gdbserver/ChangeLog:

        * configure.ac: Add checks for readlink function and declaration.
        * configure: Regenerate.
        * config.in: Regenerate.

        * Makefile.in (SFILES): Add gdb_readlink.c.
        (OBS): Add gdb_readlink.o.
        (gdb_readlink_h): New variable.
        (gdb_readlink.o): New rule.

Tested on x86-windows with AdaCore's testsuite, although I think
only GNU/Linux targets would use the packet that would use this
routine.

On a side note: If this patch is installed, I think it should be
possible to adjust the inf_child_fileio_readlink code to remove
all #ifdefs. It's a tad trickier because of the dependency on
MAXPATHLEN (I think we should have a portable MAXPATH, GDB_MAXPATH,
and be done with those silly ifdefs and inconsistent uses of MAXPATH
vs MAXPATHLEN vs ...).

OK to commit?
Thanks,
-- 
Joel

---
 gdb/common/gdb_readlink.c  |   37 +++++++++++++++++++++++++++++++++++++
 gdb/common/gdb_readlink.h  |   36 ++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/Makefile.in  |    9 +++++++--
 gdb/gdbserver/config.in    |    7 +++++++
 gdb/gdbserver/configure    |   12 +++++++++++-
 gdb/gdbserver/configure.ac |    4 ++--
 6 files changed, 100 insertions(+), 5 deletions(-)
 create mode 100644 gdb/common/gdb_readlink.c
 create mode 100644 gdb/common/gdb_readlink.h

diff --git a/gdb/common/gdb_readlink.c b/gdb/common/gdb_readlink.c
new file mode 100644
index 0000000..2e5c8e3
--- /dev/null
+++ b/gdb/common/gdb_readlink.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdb_readlink.h"
+
+#ifndef HAVE_READLINK
+#include <errno.h>
+
+/* Provide a dummy version of readlink on the systems that do not
+   support this routine.
+
+   This function return -1 (error) and sets errno to ENOSYS to indicate
+   that this function is not implemented.  The caller should be prepared
+   to recover from that.  */
+
+ssize_t
+readlink(const char *path, char *buf, size_t bufsiz)
+{
+  errno = EINVAL;
+  return -1;
+}
+#endif
+
diff --git a/gdb/common/gdb_readlink.h b/gdb/common/gdb_readlink.h
new file mode 100644
index 0000000..6acc6fd
--- /dev/null
+++ b/gdb/common/gdb_readlink.h
@@ -0,0 +1,36 @@
+/* Portable access to `readlink'.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_READLINK_H
+#define GDB_READLINK_H
+
+/* If readlink is available, it should be declared in "unistd.h".  */
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#include <sys/types.h>
+#include "config.h"
+
+#ifndef HAVE_DECL_READLINK
+extern ssize_t readlink (const char *path, char *buf, size_t bufsiz);
+#endif
+
+#endif
+
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 6ccf5ae..1e28ef2 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -125,7 +125,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c \
 	$(srcdir)/hostio.c $(srcdir)/hostio-errno.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c \
+	$(srcdir)/common/gdb_readlink.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -137,7 +138,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 OBS = inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
 	utils.o version.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
-	xml-utils.o common-utils.o ptid.o buffer.o \
+	xml-utils.o common-utils.o ptid.o buffer.o gdb_readlink.o \
 	$(XML_BUILTIN) \
 	$(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
@@ -345,6 +346,7 @@ server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
 		$(srcdir)/../common/gdb_locale.h \
 		$(ptid_h) \
 		$(signals_h)
+gdb_readlink_h = $(srcdir)/../common/gdb_readlink.h
 
 linux_low_h = $(srcdir)/linux-low.h
 
@@ -423,6 +425,9 @@ ptid.o: ../common/ptid.c $(ptid_h)
 buffer.o: ../common/buffer.c $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+gdb_readlink.o: ../common/gdb_readlink.c $(gdb_readlink_h)
+	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
+
 # We build memmem.c without -Werror because this file is not under
 # our control.  On LynxOS, the compiler generates some warnings
 # because str-two-way.h uses a constant (MAX_SIZE) whose definition
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index a9472ea..b2a356d 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -30,6 +30,10 @@
    */
 #undef HAVE_DECL_PERROR
 
+/* Define to 1 if you have the declaration of `readlink', and to 0 if you
+   don't. */
+#undef HAVE_DECL_READLINK
+
 /* Define to 1 if you have the declaration of `strerror', and to 0 if you
    don't. */
 #undef HAVE_DECL_STRERROR
@@ -128,6 +132,9 @@
 /* Define to 1 if you have the `pwrite' function. */
 #undef HAVE_PWRITE
 
+/* Define to 1 if you have the `readlink' function. */
+#undef HAVE_READLINK
+
 /* Define to 1 if you have the <sgtty.h> header file. */
 #undef HAVE_SGTTY_H
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index d92a00f..b014254 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4163,7 +4163,7 @@ fi
 
 done
 
-for ac_func in pread pwrite pread64
+for ac_func in pread pwrite pread64 readlink
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -4458,6 +4458,16 @@ fi
 cat >>confdefs.h <<_ACEOF
 #define HAVE_DECL_VSNPRINTF $ac_have_decl
 _ACEOF
+ac_fn_c_check_decl "$LINENO" "readlink" "ac_cv_have_decl_readlink" "$ac_includes_default"
+if test "x$ac_cv_have_decl_readlink" = x""yes; then :
+  ac_have_decl=1
+else
+  ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_READLINK $ac_have_decl
+_ACEOF
 
 
 ac_fn_c_check_type "$LINENO" "socklen_t" "ac_cv_type_socklen_t" "#include <sys/types.h>
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 30666ec..0382430 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -43,7 +43,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
 		 netinet/tcp.h arpa/inet.h sys/wait.h)
-AC_CHECK_FUNCS(pread pwrite pread64)
+AC_CHECK_FUNCS(pread pwrite pread64 readlink)
 AC_REPLACE_FUNCS(memmem vasprintf vsnprintf)
 
 # Check for UST
@@ -161,7 +161,7 @@ AC_TRY_LINK([
   [AC_MSG_RESULT(no)])
 fi
 
-AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
+AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf, readlink])
 
 AC_CHECK_TYPES(socklen_t, [], [],
 [#include <sys/types.h>
-- 
1.7.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFA/gdbserver] Provide dummy readlink on systems where routine is not available.
  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
  0 siblings, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2012-02-03 11:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: palves, Joel Brobecker

Hello,

No sooner did I send the patch that I realized that I forgot to include
the new .h file in gdbserver/hostio.c. No biggie, as compilation warnings
do not seem to be fatal in gdbserver, but easy to fix.

Here is a new version of the patch.

-- 
Joel

This fixes the gdbserver build failure on Windows hosts due to
a missing readlink (not implemented on this system). It does so
by providing a dummy readlink for systems, like Windows, which
do not provide one.  The dummy version returns -1 and sets errno
to ENOSYS (function not implemented).

gdb/ChangeLog:

        * common/gdb_readlink.h, common/gdb_readlink.c: New files.

gdb/gdbserver/ChangeLog:

        * configure.ac: Add checks for readlink function and declaration.
        * configure: Regenerate.
        * config.in: Regenerate.
        * Makefile.in (SFILES): Add gdb_readlink.c.
        (OBS): Add gdb_readlink.o.
        (gdb_readlink_h): New variable.
        (gdb_readlink.o): New rule.
        * hostio.c: #include "gdb_readlink.h".
---
 gdb/common/gdb_readlink.c  |   37 +++++++++++++++++++++++++++++++++++++
 gdb/common/gdb_readlink.h  |   36 ++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/Makefile.in  |    9 +++++++--
 gdb/gdbserver/config.in    |    7 +++++++
 gdb/gdbserver/configure    |   12 +++++++++++-
 gdb/gdbserver/configure.ac |    4 ++--
 gdb/gdbserver/hostio.c     |    1 +
 7 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 gdb/common/gdb_readlink.c
 create mode 100644 gdb/common/gdb_readlink.h

diff --git a/gdb/common/gdb_readlink.c b/gdb/common/gdb_readlink.c
new file mode 100644
index 0000000..2e5c8e3
--- /dev/null
+++ b/gdb/common/gdb_readlink.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdb_readlink.h"
+
+#ifndef HAVE_READLINK
+#include <errno.h>
+
+/* Provide a dummy version of readlink on the systems that do not
+   support this routine.
+
+   This function return -1 (error) and sets errno to ENOSYS to indicate
+   that this function is not implemented.  The caller should be prepared
+   to recover from that.  */
+
+ssize_t
+readlink(const char *path, char *buf, size_t bufsiz)
+{
+  errno = EINVAL;
+  return -1;
+}
+#endif
+
diff --git a/gdb/common/gdb_readlink.h b/gdb/common/gdb_readlink.h
new file mode 100644
index 0000000..6acc6fd
--- /dev/null
+++ b/gdb/common/gdb_readlink.h
@@ -0,0 +1,36 @@
+/* Portable access to `readlink'.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_READLINK_H
+#define GDB_READLINK_H
+
+/* If readlink is available, it should be declared in "unistd.h".  */
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#include <sys/types.h>
+#include "config.h"
+
+#ifndef HAVE_DECL_READLINK
+extern ssize_t readlink (const char *path, char *buf, size_t bufsiz);
+#endif
+
+#endif
+
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 485e031..cd83507 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -125,7 +125,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c \
 	$(srcdir)/hostio.c $(srcdir)/hostio-errno.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c \
+	$(srcdir)/common/gdb_readlink.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -137,7 +138,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 OBS = inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
 	utils.o version.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
-	xml-utils.o common-utils.o ptid.o buffer.o \
+	xml-utils.o common-utils.o ptid.o buffer.o gdb_readlink.o \
 	$(XML_BUILTIN) \
 	$(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
@@ -343,6 +344,7 @@ server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
 		$(srcdir)/../common/gdb_locale.h \
 		$(ptid_h) \
 		$(signals_h)
+gdb_readlink_h = $(srcdir)/../common/gdb_readlink.h
 
 linux_low_h = $(srcdir)/linux-low.h
 
@@ -421,6 +423,9 @@ ptid.o: ../common/ptid.c $(ptid_h)
 buffer.o: ../common/buffer.c $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+gdb_readlink.o: ../common/gdb_readlink.c $(gdb_readlink_h)
+	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
+
 # We build memmem.c without -Werror because this file is not under
 # our control.  On LynxOS, the compiler generates some warnings
 # because str-two-way.h uses a constant (MAX_SIZE) whose definition
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index a9472ea..b2a356d 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -30,6 +30,10 @@
    */
 #undef HAVE_DECL_PERROR
 
+/* Define to 1 if you have the declaration of `readlink', and to 0 if you
+   don't. */
+#undef HAVE_DECL_READLINK
+
 /* Define to 1 if you have the declaration of `strerror', and to 0 if you
    don't. */
 #undef HAVE_DECL_STRERROR
@@ -128,6 +132,9 @@
 /* Define to 1 if you have the `pwrite' function. */
 #undef HAVE_PWRITE
 
+/* Define to 1 if you have the `readlink' function. */
+#undef HAVE_READLINK
+
 /* Define to 1 if you have the <sgtty.h> header file. */
 #undef HAVE_SGTTY_H
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index d92a00f..b014254 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4163,7 +4163,7 @@ fi
 
 done
 
-for ac_func in pread pwrite pread64
+for ac_func in pread pwrite pread64 readlink
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -4458,6 +4458,16 @@ fi
 cat >>confdefs.h <<_ACEOF
 #define HAVE_DECL_VSNPRINTF $ac_have_decl
 _ACEOF
+ac_fn_c_check_decl "$LINENO" "readlink" "ac_cv_have_decl_readlink" "$ac_includes_default"
+if test "x$ac_cv_have_decl_readlink" = x""yes; then :
+  ac_have_decl=1
+else
+  ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_READLINK $ac_have_decl
+_ACEOF
 
 
 ac_fn_c_check_type "$LINENO" "socklen_t" "ac_cv_type_socklen_t" "#include <sys/types.h>
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 30666ec..0382430 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -43,7 +43,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
 		 netinet/tcp.h arpa/inet.h sys/wait.h)
-AC_CHECK_FUNCS(pread pwrite pread64)
+AC_CHECK_FUNCS(pread pwrite pread64 readlink)
 AC_REPLACE_FUNCS(memmem vasprintf vsnprintf)
 
 # Check for UST
@@ -161,7 +161,7 @@ AC_TRY_LINK([
   [AC_MSG_RESULT(no)])
 fi
 
-AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
+AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf, readlink])
 
 AC_CHECK_TYPES(socklen_t, [], [],
 [#include <sys/types.h>
diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index 34e4fa8..370c958 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -20,6 +20,7 @@
 
 #include "server.h"
 #include "gdb/fileio.h"
+#include "gdb_readlink.h"
 
 #include <fcntl.h>
 #include <limits.h>
-- 
1.7.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFA/gdbserver] Provide dummy readlink on systems where routine is not available.
  2012-02-03 11:56       ` Joel Brobecker
@ 2012-02-03 12:31         ` Joel Brobecker
  2012-02-03 13:11           ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2012-02-03 12:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: palves

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

It looks like I'm doing sloppy work, today.

Another revised version, with 2 small changes:
  - The pre-processor condition used to determine whether we need to
    declare readlink in gdb_readlink.h or not was wrong. Somehow,
    HAVE_ macros and HAVE_DECL_ macros are handled differently.
  - I forgot to update hostio.c's dependency on gdb_readlink.h
    in the Makefile.

Both issues now fixed, and new patch attached.

I'm going to stop here for the day.

-- 
Joel

[-- Attachment #2: gdb-readlink-gdbserver-v3.diff --]
[-- Type: text/x-diff, Size: 8486 bytes --]

commit 8c8b9045fe33cc09d5ee32ae51c5490adbba2e41
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Fri Feb 3 14:53:38 2012 +0400

    Provide dummy readlink on systems where routine is not available.
    
    This fixes the gdbserver build failure on Windows hosts due to
    a missing readlink (not implemented on this system). It does so
    by providing a dummy readlink for systems, like Windows, which
    do not provide one.  The dummy version returns -1 and sets errno
    to ENOSYS (function not implemented).
    
    gdb/ChangeLog:
    
            * common/gdb_readlink.h, common/gdb_readlink.c: New files.
    
    gdb/gdbserver/ChangeLog:
    
            * configure.ac: Add checks for readlink function and declaration.
            * configure: Regenerate.
            * config.in: Regenerate.
            * Makefile.in (SFILES): Add gdb_readlink.c.
            (OBS): Add gdb_readlink.o.
            (gdb_readlink_h): New variable.
            (hostio.o): Add dependency on gdb_readlink.h.
            (gdb_readlink.o): New rule.
            * hostio.c: #include "gdb_readlink.h".

diff --git a/gdb/common/gdb_readlink.c b/gdb/common/gdb_readlink.c
new file mode 100644
index 0000000..2e5c8e3
--- /dev/null
+++ b/gdb/common/gdb_readlink.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdb_readlink.h"
+
+#ifndef HAVE_READLINK
+#include <errno.h>
+
+/* Provide a dummy version of readlink on the systems that do not
+   support this routine.
+
+   This function return -1 (error) and sets errno to ENOSYS to indicate
+   that this function is not implemented.  The caller should be prepared
+   to recover from that.  */
+
+ssize_t
+readlink(const char *path, char *buf, size_t bufsiz)
+{
+  errno = EINVAL;
+  return -1;
+}
+#endif
+
diff --git a/gdb/common/gdb_readlink.h b/gdb/common/gdb_readlink.h
new file mode 100644
index 0000000..f8afd55
--- /dev/null
+++ b/gdb/common/gdb_readlink.h
@@ -0,0 +1,36 @@
+/* Portable access to `readlink'.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_READLINK_H
+#define GDB_READLINK_H
+
+/* If readlink is available, it should be declared in "unistd.h".  */
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#include <sys/types.h>
+#include "config.h"
+
+#if !HAVE_DECL_READLINK
+extern ssize_t readlink (const char *path, char *buf, size_t bufsiz);
+#endif
+
+#endif
+
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 6ccf5ae..2c2c8d3 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -125,7 +125,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c \
 	$(srcdir)/hostio.c $(srcdir)/hostio-errno.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c \
+	$(srcdir)/common/gdb_readlink.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -137,7 +138,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 OBS = inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
 	utils.o version.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
-	xml-utils.o common-utils.o ptid.o buffer.o \
+	xml-utils.o common-utils.o ptid.o buffer.o gdb_readlink.o \
 	$(XML_BUILTIN) \
 	$(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
@@ -345,6 +346,7 @@ server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
 		$(srcdir)/../common/gdb_locale.h \
 		$(ptid_h) \
 		$(signals_h)
+gdb_readlink_h = $(srcdir)/../common/gdb_readlink.h
 
 linux_low_h = $(srcdir)/linux-low.h
 
@@ -387,7 +389,7 @@ amd64-linux-ipa.o : amd64-linux.c $(regdef_h)
 	$(CC) -c $(IPAGENT_CFLAGS) $< -o amd64-linux-ipa.o
 
 event-loop.o: event-loop.c $(server_h)
-hostio.o: hostio.c $(server_h)
+hostio.o: hostio.c $(server_h) $(gdb_readlink_h)
 hostio-errno.o: hostio-errno.c $(server_h)
 inferiors.o: inferiors.c $(server_h)
 mem-break.o: mem-break.c $(server_h)
@@ -423,6 +425,9 @@ ptid.o: ../common/ptid.c $(ptid_h)
 buffer.o: ../common/buffer.c $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+gdb_readlink.o: ../common/gdb_readlink.c $(gdb_readlink_h)
+	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
+
 # We build memmem.c without -Werror because this file is not under
 # our control.  On LynxOS, the compiler generates some warnings
 # because str-two-way.h uses a constant (MAX_SIZE) whose definition
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index a9472ea..b2a356d 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -30,6 +30,10 @@
    */
 #undef HAVE_DECL_PERROR
 
+/* Define to 1 if you have the declaration of `readlink', and to 0 if you
+   don't. */
+#undef HAVE_DECL_READLINK
+
 /* Define to 1 if you have the declaration of `strerror', and to 0 if you
    don't. */
 #undef HAVE_DECL_STRERROR
@@ -128,6 +132,9 @@
 /* Define to 1 if you have the `pwrite' function. */
 #undef HAVE_PWRITE
 
+/* Define to 1 if you have the `readlink' function. */
+#undef HAVE_READLINK
+
 /* Define to 1 if you have the <sgtty.h> header file. */
 #undef HAVE_SGTTY_H
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index d92a00f..b014254 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4163,7 +4163,7 @@ fi
 
 done
 
-for ac_func in pread pwrite pread64
+for ac_func in pread pwrite pread64 readlink
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -4458,6 +4458,16 @@ fi
 cat >>confdefs.h <<_ACEOF
 #define HAVE_DECL_VSNPRINTF $ac_have_decl
 _ACEOF
+ac_fn_c_check_decl "$LINENO" "readlink" "ac_cv_have_decl_readlink" "$ac_includes_default"
+if test "x$ac_cv_have_decl_readlink" = x""yes; then :
+  ac_have_decl=1
+else
+  ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_READLINK $ac_have_decl
+_ACEOF
 
 
 ac_fn_c_check_type "$LINENO" "socklen_t" "ac_cv_type_socklen_t" "#include <sys/types.h>
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 30666ec..0382430 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -43,7 +43,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
 		 netinet/tcp.h arpa/inet.h sys/wait.h)
-AC_CHECK_FUNCS(pread pwrite pread64)
+AC_CHECK_FUNCS(pread pwrite pread64 readlink)
 AC_REPLACE_FUNCS(memmem vasprintf vsnprintf)
 
 # Check for UST
@@ -161,7 +161,7 @@ AC_TRY_LINK([
   [AC_MSG_RESULT(no)])
 fi
 
-AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf])
+AC_CHECK_DECLS([strerror, perror, memmem, vasprintf, vsnprintf, readlink])
 
 AC_CHECK_TYPES(socklen_t, [], [],
 [#include <sys/types.h>
diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index 34e4fa8..370c958 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -20,6 +20,7 @@
 
 #include "server.h"
 #include "gdb/fileio.h"
+#include "gdb_readlink.h"
 
 #include <fcntl.h>
 #include <limits.h>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFA/gdbserver] Provide dummy readlink on systems where routine is not available.
  2012-02-03 12:31         ` Joel Brobecker
@ 2012-02-03 13:11           ` Eli Zaretskii
  2012-02-03 13:26             ` Joel Brobecker
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2012-02-03 13:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches, palves

> Date: Fri, 3 Feb 2012 16:30:30 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: palves@redhat.com
> 
> Another revised version, with 2 small changes:
>   - The pre-processor condition used to determine whether we need to
>     declare readlink in gdb_readlink.h or not was wrong. Somehow,
>     HAVE_ macros and HAVE_DECL_ macros are handled differently.
>   - I forgot to update hostio.c's dependency on gdb_readlink.h
>     in the Makefile.
> 
> Both issues now fixed, and new patch attached.

If memory serves, Pedro was against this when I suggested it.

> +   This function return -1 (error) and sets errno to ENOSYS to indicate
> +   that this function is not implemented.  The caller should be prepared
> +   to recover from that.  */

But the function in fact returns EINVAL:

> +ssize_t
> +readlink(const char *path, char *buf, size_t bufsiz)
> +{
> +  errno = EINVAL;
> +  return -1;
> +}

I cannot resist re-iterating my original suggestion: test the file for
existence, return ENOENT if it doesn't exist, and otherwise return
EINVAL, meaning that the file is not a link.

Or just use the readlink module from gnulib, which does exactly that.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFA/gdbserver] Provide dummy readlink on systems where routine is not available.
  2012-02-03 13:11           ` Eli Zaretskii
@ 2012-02-03 13:26             ` Joel Brobecker
  0 siblings, 0 replies; 14+ messages in thread
From: Joel Brobecker @ 2012-02-03 13:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, palves

> If memory serves, Pedro was against this when I suggested it.

Do you mean returning EINVAL, or having a dummy readlink?

> > +   This function return -1 (error) and sets errno to ENOSYS to indicate
> > +   that this function is not implemented.  The caller should be prepared
> > +   to recover from that.  */
> 
> But the function in fact returns EINVAL:

<swear-word>I'll fix that, but another day, when I seem to have
a functioning brain.

> I cannot resist re-iterating my original suggestion: test the file for
> existence, return ENOENT if it doesn't exist, and otherwise return
> EINVAL, meaning that the file is not a link.
> 
> Or just use the readlink module from gnulib, which does exactly that.

The first suggestion is too much work that has already been done
by gnulib. The second is too much work for the time I have right
now because gnulib hasn't been formally introduced in gdbserver.
I tried doing just that many moons ago and failed miserably.
I think I have better chances today, but I am still not confident
at all. Part of the problem comes from the fact that gnulib more
or less assumes that the project using it uses automake, which
is not the case for neither GDB nor GDBserver.

This patch has the advantage of being simple and fixing a build failure
that has been lingering since Jan 20th. If peopler prefer, I can
implement the "#if noreadlink ... #else return ENOSYS" approach,
which is fewer changes, but less appealing (IMO).

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFA/commit] gdbserver: return ENOSYS if readlink not supported.
  2012-01-27 10:28   ` Pedro Alves
  2012-01-27 12:22     ` Eli Zaretskii
  2012-02-03 11:47     ` [RFA/gdbserver] Provide dummy readlink on systems where routine is not available Joel Brobecker
@ 2012-02-09 16:32     ` Joel Brobecker
  2012-02-09 16:41       ` Pedro Alves
  2 siblings, 1 reply; 14+ messages in thread
From: Joel Brobecker @ 2012-02-09 16:32 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, asmwarrior, Yao Qi, Joel Brobecker

Hi Pedro,

Given that this problem has been lingering for 3 weeks, I've decided
to just KISS.  This reproduces on the GDBserver side what GDB does
when readlink is not supported. Does this look good to you?

Thanks!

gdb/ChangeLog:

        * configure.ac: Add readlink to AC_CHECK_FUNCS list.
        * configure, config.in: Regenerate.
        * hostio.c: Provide an alternate implementation if HAVE_READLINK
        is not defined.

Tested on x86_64-linux and x86-windows (by simply rebuilding and verifying
that the correct section of the #if/#else is taken). I'd like to commit
sometime today if possible - people keep reporting this problem.

---
 gdb/gdbserver/config.in    |    3 +++
 gdb/gdbserver/configure    |    2 +-
 gdb/gdbserver/configure.ac |    2 +-
 gdb/gdbserver/hostio.c     |    4 ++++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index a9472ea..7fa3b5b 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -128,6 +128,9 @@
 /* Define to 1 if you have the `pwrite' function. */
 #undef HAVE_PWRITE
 
+/* Define to 1 if you have the `readlink' function. */
+#undef HAVE_READLINK
+
 /* Define to 1 if you have the <sgtty.h> header file. */
 #undef HAVE_SGTTY_H
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index d92a00f..e251844 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4163,7 +4163,7 @@ fi
 
 done
 
-for ac_func in pread pwrite pread64
+for ac_func in pread pwrite pread64 readlink
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 30666ec..7c86cd4 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -43,7 +43,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl
 		 errno.h fcntl.h signal.h sys/file.h malloc.h dnl
 		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
 		 netinet/tcp.h arpa/inet.h sys/wait.h)
-AC_CHECK_FUNCS(pread pwrite pread64)
+AC_CHECK_FUNCS(pread pwrite pread64 readlink)
 AC_REPLACE_FUNCS(memmem vasprintf vsnprintf)
 
 # Check for UST
diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
index 34e4fa8..03aab58 100644
--- a/gdb/gdbserver/hostio.c
+++ b/gdb/gdbserver/hostio.c
@@ -459,6 +459,7 @@ handle_unlink (char *own_buf)
 static void
 handle_readlink (char *own_buf, int *new_packet_len)
 {
+#if defined (HAVE_READLINK)
   char filename[PATH_MAX], linkname[PATH_MAX];
   char *p;
   int ret, bytes_sent;
@@ -485,6 +486,9 @@ handle_readlink (char *own_buf, int *new_packet_len)
      to return a partial response, but simply fail.  */
   if (bytes_sent < ret)
     sprintf (own_buf, "F-1,%x", FILEIO_ENAMETOOLONG);
+#else /* ! HAVE_READLINK */
+    sprintf (own_buf, "F-1,%x", FILEIO_ENOSYS);
+#endif
 }
 
 /* Handle all the 'F' file transfer packets.  */
-- 
1.7.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFA/commit] gdbserver: return ENOSYS if readlink not supported.
  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
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2012-02-09 16:41 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches, asmwarrior, Yao Qi

On 02/09/2012 04:31 PM, Joel Brobecker wrote:

> Given that this problem has been lingering for 3 weeks, I've decided
> to just KISS.  This reproduces on the GDBserver side what GDB does
> when readlink is not supported. Does this look good to you?

Yes, it does.  Thanks!

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFA/commit] gdbserver: return ENOSYS if readlink not supported.
  2012-02-09 16:41       ` Pedro Alves
@ 2012-02-09 17:32         ` Joel Brobecker
  0 siblings, 0 replies; 14+ messages in thread
From: Joel Brobecker @ 2012-02-09 17:32 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, asmwarrior, Yao Qi

> > Given that this problem has been lingering for 3 weeks, I've decided
> > to just KISS.  This reproduces on the GDBserver side what GDB does
> > when readlink is not supported. Does this look good to you?
> 
> Yes, it does.  Thanks!

Awesome, thanks! Patch now checked in.

For Eli, I am not discarding his feedback, but in the interest of getting
the build failure out of the way, decided to go for simple. If we'd like
to improve the situation, I think the best approach is to use gnulib's
readlink. But given that this appears to be used on Linux systems only,
this work would be purely accademic at this point. So I think the effort
can be defered for now.

-- 
Joel


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-02-09 17:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-27  3:20 fix build error on MinGW (HAVE_READLINK) undefined asmwarrior
2012-01-27  9:41 ` Eli Zaretskii
2012-01-27 10:28   ` Pedro Alves
2012-01-27 12:22     ` Eli Zaretskii
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox