[RFC] Missing sleep function for mingw hosts

[RFC] Missing sleep function for mingw hosts

[Pierre Muller]

mingw does not have a sleep function in its library.
This prevents currently successful compilation of
mingw64 with --enable-targets=all option.


I found that there is a substitute for sleep in gnulib:
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/sleep.c;hb=d60
f3b0c6b0f93a601acd1cfd3923f94ca05abb0

  What is the rule to add new parts into gnulib subdirectory?
Would inclusion of gnulib sleep.c source code
be possible to fix the above problem?

 
Pierre

Re: [RFC] Missing sleep function for mingw hosts

[Eli Zaretskii]

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Date: Tue, 29 Mar 2011 09:36:46 +0200
> 
> mingw does not have a sleep function in its library.
> This prevents currently successful compilation of
> mingw64 with --enable-targets=all option.

You say "mingw64" -- does that mean it does compile with mingw32?  If
so, how come it fails with the 64-bit build?

> I found that there is a substitute for sleep in gnulib:
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/sleep.c;hb=d60
> f3b0c6b0f93a601acd1cfd3923f94ca05abb0
> 
>   What is the rule to add new parts into gnulib subdirectory?
> Would inclusion of gnulib sleep.c source code
> be possible to fix the above problem?

I'd rather we had a replacement in win32-nat.c, instead of importing
gobs of gnulib stuff needed to provide such a trivial replacement.
gnulib is GPL, so we can simply copy the code into our sources.

RE: [RFC] Missing sleep function for mingw hosts

[Pierre Muller]

  Hi Eli,
thanks for your comments.

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : mardi 29 mars 2011 14:37
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC] Missing sleep function for mingw hosts
> 
> > From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> > Date: Tue, 29 Mar 2011 09:36:46 +0200
> >
> > mingw does not have a sleep function in its library.
> > This prevents currently successful compilation of
> > mingw64 with --enable-targets=all option.
> 
> You say "mingw64" -- does that mean it does compile with mingw32?  If
> so, how come it fails with the 64-bit build?
  No, it's just the configuration that I used,
but this failure also appears for mingw32.
../../../archer/gdb/remote-mips.c: In function `mips_enter_debug':
../../../archer/gdb/remote-mips.c:1353: warning: implicit declaration of
functio
n `sleep'
make: *** [remote-mips.o] Error 1 

> > I found that there is a substitute for sleep in gnulib:
> >
>
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/sleep.c;hb=d60
> > f3b0c6b0f93a601acd1cfd3923f94ca05abb0
> >
> >   What is the rule to add new parts into gnulib subdirectory?
> > Would inclusion of gnulib sleep.c source code
> > be possible to fix the above problem?
> 
> I'd rather we had a replacement in win32-nat.c, instead of importing
> gobs of gnulib stuff needed to provide such a trivial replacement.
> gnulib is GPL, so we can simply copy the code into our sources.

  This is not really the right place:
the problem triggers due to some other target,
(remote-mips.c line 1353)
so the code should be also available if only that target is selected
(i.e. without windows-nat.c ).
  The best place in my opinion would be mingw-hdep.c,
the only thing is that this would mean adding a mingw-hdep.h header
and inserting that header into remote-mips.c source...
  The big difference with using gnulib code is that the same failure
would not appear again the next time someone adds a call to sleep
somewhere else inside the GDB sources.

Pierre

Re: [RFC] Missing sleep function for mingw hosts

[Pedro Alves]

On Tuesday 29 March 2011 14:00:32, Pierre Muller wrote:
>   Hi Eli,
> > > I found that there is a substitute for sleep in gnulib:
> > >
> >
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/sleep.c;hb=d60
> > > f3b0c6b0f93a601acd1cfd3923f94ca05abb0
> > >
> > >   What is the rule to add new parts into gnulib subdirectory?
> > > Would inclusion of gnulib sleep.c source code
> > > be possible to fix the above problem?
> > 
> > I'd rather we had a replacement in win32-nat.c, instead of importing
> > gobs of gnulib stuff needed to provide such a trivial replacement.
> > gnulib is GPL, so we can simply copy the code into our sources.

gnulib has a mechanism (which we are already using) where
we just import the modules we need.  We don't import the whole thing.  We
currently import the memmem module.  We would just need to import the
"sleep" module in addition (and whatever dependencies that
may have --- which are handled automatically by gnulib's import script).

I have no clue why that `sleep' is necessary, but in any case, can't
we use already existing `gdb_usleep' function instead and be done
with it?

-- 
Pedro Alves

Re: [RFC] Missing sleep function for mingw hosts

[Kai Tietz]

2011/3/29 Pedro Alves <pedro@codesourcery.com>:
> On Tuesday 29 March 2011 14:00:32, Pierre Muller wrote:
>>   Hi Eli,
>> > > I found that there is a substitute for sleep in gnulib:
>> > >
>> >
>> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/sleep.c;hb=d60
>> > > f3b0c6b0f93a601acd1cfd3923f94ca05abb0
>> > >
>> > >   What is the rule to add new parts into gnulib subdirectory?
>> > > Would inclusion of gnulib sleep.c source code
>> > > be possible to fix the above problem?
>> >
>> > I'd rather we had a replacement in win32-nat.c, instead of importing
>> > gobs of gnulib stuff needed to provide such a trivial replacement.
>> > gnulib is GPL, so we can simply copy the code into our sources.
>
> gnulib has a mechanism (which we are already using) where
> we just import the modules we need.  We don't import the whole thing.  We
> currently import the memmem module.  We would just need to import the
> "sleep" module in addition (and whatever dependencies that
> may have --- which are handled automatically by gnulib's import script).
>
> I have no clue why that `sleep' is necessary, but in any case, can't
> we use already existing `gdb_usleep' function instead and be done
> with it?
>
> --
> Pedro Alves
>

Yes, this sounds to me like a good solution.  The usleep function is
available for mingw runtime, so this looks simpler to me, too.

Regards,
Kai

[RFA] use gdb_usleep instead of sleep in remote-mips.c (was Missing sleep function for mingw hosts)

[Pierre Muller]

> I have no clue why that `sleep' is necessary, but in any case, can't
> we use already existing `gdb_usleep' function instead and be done
> with it?

  Simply because I didn't know that gdb_usleep even existed...
This makes things much easier indeed.

Is the patch below OK?

Pierre


2011-03-29  Pierre Muller  <muller@ics.u-strasbg.fr>

	Fix mingw compilation with --enable-targets=all.
	* remote-mips.c (gdb_usleep.h): Include header.
	(mips_enter_debug): Use gdb_usleep instead of sleep.

Index: remote-mips.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-mips.c,v
retrieving revision 1.120
diff -u -p -r1.120 remote-mips.c
--- remote-mips.c	16 Mar 2011 17:59:02 -0000	1.120
+++ remote-mips.c	29 Mar 2011 14:52:12 -0000
@@ -33,6 +33,7 @@
 #include "exceptions.h"
 #include "gdb_string.h"
 #include "gdb_stat.h"
+#include "gdb_usleep.h"
 #include "regcache.h"
 #include <ctype.h>
 #include "mips-tdep.h"
@@ -1350,7 +1351,7 @@ mips_enter_debug (void)
   else				/* Assume IDT monitor by default.  */
     mips_send_command ("db tty0\r", 0);
 
-  sleep (1);
+  gdb_usleep (1000000);
   serial_write (mips_desc, "\r", sizeof "\r" - 1);
 
   /* We don't need to absorb any spurious characters here, since the

Re: [RFA] use gdb_usleep instead of sleep in remote-mips.c (was Missing sleep function for mingw hosts)

[Joel Brobecker]

> 2011-03-29  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	Fix mingw compilation with --enable-targets=all.
> 	* remote-mips.c (gdb_usleep.h): Include header.
> 	(mips_enter_debug): Use gdb_usleep instead of sleep.

Looks good to me...

-- 
Joel

RE: [RFA] use gdb_usleep instead of sleep in remote-mips.c (was Missing sleep function for mingw hosts)

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : mardi 29 mars 2011 17:43
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org; 'Eli Zaretskii'
> Objet : Re: [RFA] use gdb_usleep instead of sleep in remote-mips.c (was
> Missing sleep function for mingw hosts)
> 
> > 2011-03-29  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	Fix mingw compilation with --enable-targets=all.
> > 	* remote-mips.c (gdb_usleep.h): Include header.
> > 	(mips_enter_debug): Use gdb_usleep instead of sleep.
> 
> Looks good to me...

Thanks for the approval,
patch committed.

Pierre

Re: [RFC] Missing sleep function for mingw hosts

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Tue, 29 Mar 2011 15:23:46 +0100
> Cc: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>,
>  "'Eli Zaretskii'" <eliz@gnu.org>
> 
> > > I'd rather we had a replacement in win32-nat.c, instead of importing
> > > gobs of gnulib stuff needed to provide such a trivial replacement.
> > > gnulib is GPL, so we can simply copy the code into our sources.
> 
> gnulib has a mechanism (which we are already using) where
> we just import the modules we need.  We don't import the whole
> thing.

I know, but "just the modules we need" are typically huge relative to
small problems they solve, this one in particular.

Anyway, I'm glad that a much more elegant solution is already
available in GDB, in the form of gdb_usleep.

Re: [RFC] Missing sleep function for mingw hosts

[Matt Rice]

On Tue, Mar 29, 2011 at 1:01 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Pedro Alves <pedro@codesourcery.com>
>> Date: Tue, 29 Mar 2011 15:23:46 +0100
>> Cc: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>,
>>  "'Eli Zaretskii'" <eliz@gnu.org>
>>
>> > > I'd rather we had a replacement in win32-nat.c, instead of importing
>> > > gobs of gnulib stuff needed to provide such a trivial replacement.
>> > > gnulib is GPL, so we can simply copy the code into our sources.
>>
>> gnulib has a mechanism (which we are already using) where
>> we just import the modules we need.  We don't import the whole
>> thing.
>
> I know, but "just the modules we need" are typically huge relative to
> small problems they solve, this one in particular.
>
> Anyway, I'm glad that a much more elegant solution is already
> available in GDB, in the form of gdb_usleep.
>

fwiw, looking at the output of:

https://gitorious.org/gnulib-module-graph/gnulib-module-graph

importing sleep brings in 2 dependencies which we do not already have,
unistd, and verify (in addition to the sleep module).

Re: [RFC] Missing sleep function for mingw hosts

[Eli Zaretskii]

> Date: Tue, 29 Mar 2011 13:21:55 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org, 
> 	pierre.muller@ics-cnrs.unistra.fr
> 
> fwiw, looking at the output of:
> 
> https://gitorious.org/gnulib-module-graph/gnulib-module-graph
> 
> importing sleep brings in 2 dependencies which we do not already have,
> unistd, and verify (in addition to the sleep module).

Aha, and all that for a function whose definition, including empty
lines and comments, is 14 lines (10 lines without that slack).
Typical.

Re: [RFC] Missing sleep function for mingw hosts

[Matt Rice]

On Tue, Mar 29, 2011 at 3:04 PM, Eli Zaretskii <eliz@gnu.org> wrote:

>
> Aha, and all that for a function whose definition, including empty
> lines and comments, is 14 lines (10 lines without that slack).
> Typical.
>

except it allows one to remove those 14 lines from gdb proper,
and call the sleep with impunity.

we & others benefit from the portability problems which we & they encounter.
like if it works with values of 49 or 29 days on various platforms.

you forgot that gdb_usleep depends on gdb_select which has 2 implementations
of either 134 or 3 lines which gnulib also has a module for.

so thats 147 lines of c code that could potentially be removed,
allowing gdb to focus on being a debugger
rather than a debugger with an ad-hoc portability layer.

Re: [RFC] Missing sleep function for mingw hosts

[Eli Zaretskii]

> Date: Tue, 29 Mar 2011 17:06:34 -0700
> From: Matt Rice <ratmice@gmail.com>
> Cc: pedro@codesourcery.com, gdb-patches@sourceware.org, 
> 	pierre.muller@ics-cnrs.unistra.fr
> 
> On Tue, Mar 29, 2011 at 3:04 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >
> > Aha, and all that for a function whose definition, including empty
> > lines and comments, is 14 lines (10 lines without that slack).
> > Typical.
> >
> 
> except it allows one to remove those 14 lines from gdb proper,
> and call the sleep with impunity.

I don't see why removing a few lines from GDB is a virtue in itself
that justifies such a sledgehammer.

> we & others benefit from the portability problems which we & they encounter.
> like if it works with values of 49 or 29 days on various platforms.

What, you mean we need in GDB to have features that sleep for 29 days?

> you forgot that gdb_usleep depends on gdb_select which has 2 implementations
> of either 134 or 3 lines which gnulib also has a module for.
> 
> so thats 147 lines of c code that could potentially be removed,
> allowing gdb to focus on being a debugger
> rather than a debugger with an ad-hoc portability layer.

I wasn't comparing gdb_usleep with gnulib's sleep.  I was comparing
gnulib's sleep with what the solution of the problem at hand should
be.  The problem is the lack of a `sleep' function in the MinGW
runtime.  The solution to this problem is a trivial 5-liner.  The
gnulib ``solution'' to the same problem is a much larger module.  (You
conveniently dismissed some of what that would bring into a package
that wants to use `sleep' on the ground that GDB already has that for
other needs.  But in judging the elegance of a solution, those other
dependencies should not be forgotten.)

Frankly, I'm amazed how people don't see the inelegance of these
massive ``solutions''.  Is that just because they conveniently allow
us to forget about the problem we needed solve?  Me, I like the
libiberty style: get a problem solved in the most direct and efficient
way that can exist.  This is what engineering is all about, IMO.

Anyway, this will be my last post on this issue, since (a) we
obviously disagree, and (b) it is pointless to argue, as a solution is
already available.