[RFA] New gdb_usleep function?

[RFA] New gdb_usleep function?

[Joel Brobecker]

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

Hello guys,

I'm working on a patch for AIX where I need to do a timed-delay.
I could do what I always do, which is call gdb_select, but I thought
that this something we do occasionally, so I think it'd be nice to
have a function that does everything for us.  This is why I came up
with gdb_usleep.

As explained in the "Limitation:" comment, the function is not perfect,
since it might return early if a signal is raised.  But I think it's
good enough for what we need to do, at least thus far.  I think it can
be enhanced by checking the time remaining, but I'm not certain that
this is completely portable.

2009-03-17  Joel Brobecker  <brobecker@adacore.com>

        Add gdb_usleep as a portable version of sleep based on gdb_select.

        * gdb_usleep.h, gdb_usleep.c: New files.
        * Makefile.in (SFILES): Add gdb_usleep.c.
        (HFILES_NO_SRCDIR): Add gdb_usleep.h.
        (COMMON_OBS): Add gdb_usleep.o.
        * ser-unix.c (hardwire_send_break): Replace call to gdb_select
        by call to gdb_usleep.

Tested in amd64-linux. Any objection?

-- 
Joel

[-- Attachment #2: gdb_usleep.diff --]
[-- Type: text/x-diff, Size: 4483 bytes --]

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 74aa72e..1ab851d 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -663,7 +663,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	valarith.c valops.c valprint.c value.c varobj.c vec.c \
 	wrapper.c \
 	xml-tdesc.c xml-support.c \
-	inferior.c
+	inferior.c gdb_usleep.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -733,7 +733,8 @@ config/sparc/nm-sol2.h config/nm-linux.h config/mips/nm-irix5.h	\
 config/rs6000/nm-rs6000.h top.h bsd-kvm.h gdb-stabs.h reggroups.h \
 annotate.h sim-regno.h dictionary.h dfp.h main.h frame-unwind.h	\
 remote-fileio.h i386-linux-tdep.h vax-tdep.h objc-lang.h \
-sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h
+sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h \
+gdb_usleep.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -814,7 +815,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	solib.o solib-null.o \
 	prologue-value.o memory-map.o xml-support.o \
 	target-descriptions.o target-memory.o xml-tdesc.o xml-builtin.o \
-	inferior.o osdata.o
+	inferior.o osdata.o gdb_usleep.o
 
 TSOBS = inflow.o
 
diff --git a/gdb/gdb_usleep.c b/gdb/gdb_usleep.c
new file mode 100644
index 0000000..e768c3d
--- /dev/null
+++ b/gdb/gdb_usleep.c
@@ -0,0 +1,37 @@
+/* Copyright (C) 2009 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 "defs.h"
+#include "gdb_select.h"
+
+int
+gdb_usleep (int usec)
+{
+  struct timeval delay;
+  int retval;
+
+  delay.tv_sec = usec / 1000000;
+  delay.tv_usec = usec % 1000000;
+  retval = gdb_select (0, 0, 0, 0, &delay);
+
+  if (retval < 0)
+    retval = -1;
+  else
+    retval = 0;
+
+  return retval;
+}
diff --git a/gdb/gdb_usleep.h b/gdb/gdb_usleep.h
new file mode 100644
index 0000000..444ffbd
--- /dev/null
+++ b/gdb/gdb_usleep.h
@@ -0,0 +1,30 @@
+/* Copyright (C) 2009 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/>.  */
+
+#if !defined(GDB_USLEEP_H)
+#define GDB_USLEEP_H
+
+/* Suspend execution for USEC microseconds.
+
+   Limitation: If a signal is raised during the delay, gdb_usleep
+   might return earlier than requested.
+
+   It returns 0 on success or -1 on error.  */
+extern int gdb_usleep (int usect);
+
+#endif /* !defined(GDB_USLEEP_H) */
+
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 07abf2b..ea2938d 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -368,16 +368,13 @@ hardwire_send_break (struct serial *scb)
 #ifdef HAVE_SGTTY
   {
     int status;
-    struct timeval timeout;
 
     status = ioctl (scb->fd, TIOCSBRK, 0);
 
     /* Can't use usleep; it doesn't exist in BSD 4.2.  */
-    /* Note that if this select() is interrupted by a signal it will not wait
-       the full length of time.  I think that is OK.  */
-    timeout.tv_sec = 0;
-    timeout.tv_usec = 250000;
-    gdb_select (0, 0, 0, 0, &timeout);
+    /* Note that if this gdb_select() is interrupted by a signal it will not
+       wait the full length of time.  I think that is OK.  */
+    gdb_usleep (250000);
     status = ioctl (scb->fd, TIOCCBRK, 0);
     return status;
   }

Re: [RFA] New gdb_usleep function?

[Jan Kratochvil]

On Tue, 17 Mar 2009 17:12:40 +0100, Joel Brobecker wrote:
> +++ b/gdb/gdb_usleep.c
...
> +#include "defs.h"
> +#include "gdb_select.h"

Missing #include "gdb_usleep.h" to crosscheck the gdb_usleep prototype.


But maybe just include it in already host-independent utils.c?


Thanks,
Jan

Re: [RFA] New gdb_usleep function?

[Joel Brobecker]

> > +++ b/gdb/gdb_usleep.c
> ...
> > +#include "defs.h"
> > +#include "gdb_select.h"
> 
> Missing #include "gdb_usleep.h" to crosscheck the gdb_usleep prototype.

You're right: gdb_usleep.c should include "gdb_usleep.h.
Thanks for catching that.

-- 
Joel

Re: [RFA] New gdb_usleep function?

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I'm working on a patch for AIX where I need to do a timed-delay.
Joel> I could do what I always do, which is call gdb_select, but I thought
Joel> that this something we do occasionally, so I think it'd be nice to
Joel> have a function that does everything for us.  This is why I came up
Joel> with gdb_usleep.

Joel> Tested in amd64-linux. Any objection?

This is not an objection, but I was wondering whether you looked at
using the gnulib usleep replacement function.  I think using gnulib
functions, when applicable, is probably preferable to writing our own,
because we can share some of the work with others.

Tom

Re: [RFA] New gdb_usleep function?

[Joel Brobecker]

> This is not an objection, but I was wondering whether you looked at
> using the gnulib usleep replacement function.

Actually, I did. AFAICT, gnulib doesn't provide gnulib...

-- 
Joel

Re: [RFA] New gdb_usleep function?

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> This is not an objection, but I was wondering whether you looked at
>> using the gnulib usleep replacement function.

Joel> Actually, I did. AFAICT, gnulib doesn't provide gnulib...

Oh, sorry ... I misread the text I saw.

thanks,
Tom

Re: [RFA] New gdb_usleep function?

[Joel Brobecker]

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

> 2009-03-17  Joel Brobecker  <brobecker@adacore.com>
> 
>         Add gdb_usleep as a portable version of sleep based on gdb_select.
> 
>         * gdb_usleep.h, gdb_usleep.c: New files.
>         * Makefile.in (SFILES): Add gdb_usleep.c.
>         (HFILES_NO_SRCDIR): Add gdb_usleep.h.
>         (COMMON_OBS): Add gdb_usleep.o.
>         * ser-unix.c (hardwire_send_break): Replace call to gdb_select
>         by call to gdb_usleep.

There were no objections, so here is what I ended up checking in.

There is one thing worth mentioning apart from the addition of the
missing #include of "gdb_usleep.h" from gdb_usleep.c that was caught
by Jan: I needed to add a #include of <sys/time.h> to get access to
struct timeval. I think it was on Tru64, IIRC.  The thing is, we
normally conditionalize the inclusion of system files like these
using #ifdef HAVE_SYS_TIME_H, but so far, we don't for <sys/time.h>.
See event-loop.c for instance. So I simply included it without
checking its availability.

-- 
Joel

[-- Attachment #2: gdb_usleep.diff --]
[-- Type: text/x-diff, Size: 4558 bytes --]

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8e19265..95b95a1 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -666,7 +666,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	valarith.c valops.c valprint.c value.c varobj.c vec.c \
 	wrapper.c \
 	xml-tdesc.c xml-support.c \
-	inferior.c
+	inferior.c gdb_usleep.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -736,7 +736,8 @@ config/sparc/nm-sol2.h config/nm-linux.h config/mips/nm-irix5.h	\
 config/rs6000/nm-rs6000.h top.h bsd-kvm.h gdb-stabs.h reggroups.h \
 annotate.h sim-regno.h dictionary.h dfp.h main.h frame-unwind.h	\
 remote-fileio.h i386-linux-tdep.h vax-tdep.h objc-lang.h \
-sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h
+sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h \
+gdb_usleep.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -817,7 +818,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	solib.o solib-null.o \
 	prologue-value.o memory-map.o xml-support.o \
 	target-descriptions.o target-memory.o xml-tdesc.o xml-builtin.o \
-	inferior.o osdata.o
+	inferior.o osdata.o gdb_usleep.o
 
 TSOBS = inflow.o
 
diff --git a/gdb/gdb_usleep.c b/gdb/gdb_usleep.c
new file mode 100644
index 0000000..dd6d460
--- /dev/null
+++ b/gdb/gdb_usleep.c
@@ -0,0 +1,41 @@
+/* Copyright (C) 2009 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 "defs.h"
+#include "gdb_usleep.h"
+#include "gdb_select.h"
+#include "gdb_usleep.h"
+
+#include <sys/time.h>
+
+int
+gdb_usleep (int usec)
+{
+  struct timeval delay;
+  int retval;
+
+  delay.tv_sec = usec / 1000000;
+  delay.tv_usec = usec % 1000000;
+  retval = gdb_select (0, 0, 0, 0, &delay);
+
+  if (retval < 0)
+    retval = -1;
+  else
+    retval = 0;
+
+  return retval;
+}
diff --git a/gdb/gdb_usleep.h b/gdb/gdb_usleep.h
new file mode 100644
index 0000000..444ffbd
--- /dev/null
+++ b/gdb/gdb_usleep.h
@@ -0,0 +1,30 @@
+/* Copyright (C) 2009 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/>.  */
+
+#if !defined(GDB_USLEEP_H)
+#define GDB_USLEEP_H
+
+/* Suspend execution for USEC microseconds.
+
+   Limitation: If a signal is raised during the delay, gdb_usleep
+   might return earlier than requested.
+
+   It returns 0 on success or -1 on error.  */
+extern int gdb_usleep (int usect);
+
+#endif /* !defined(GDB_USLEEP_H) */
+
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 07abf2b..ea2938d 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -368,16 +368,13 @@ hardwire_send_break (struct serial *scb)
 #ifdef HAVE_SGTTY
   {
     int status;
-    struct timeval timeout;
 
     status = ioctl (scb->fd, TIOCSBRK, 0);
 
     /* Can't use usleep; it doesn't exist in BSD 4.2.  */
-    /* Note that if this select() is interrupted by a signal it will not wait
-       the full length of time.  I think that is OK.  */
-    timeout.tv_sec = 0;
-    timeout.tv_usec = 250000;
-    gdb_select (0, 0, 0, 0, &timeout);
+    /* Note that if this gdb_select() is interrupted by a signal it will not
+       wait the full length of time.  I think that is OK.  */
+    gdb_usleep (250000);
     status = ioctl (scb->fd, TIOCCBRK, 0);
     return status;
   }