[5.2.1] quiet warnings for gdbreplay.c

[5.2.1] quiet warnings for gdbreplay.c

[David O'Brien]

OK to apply?

2002-06-27  David O'Brien  <obrien@FreeBSD.org>

	* gdbserver/gdbreplay.c:  Include stdlib.h, unistd.h, and string.h
	if they exist.  Static'ize internal functions to quiet warnings.

Index: gdbserver/gdbreplay.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/gdbreplay.c,v
retrieving revision 1.4.10.1
diff -u -r1.4.10.1 gdbreplay.c
--- gdbserver/gdbreplay.c	29 Mar 2002 19:03:34 -0000	1.4.10.1
+++ gdbserver/gdbreplay.c	28 Jun 2002 03:46:28 -0000
@@ -30,6 +30,15 @@
 #include <ctype.h>
 #include <fcntl.h>
 #include <errno.h>
+#ifdef HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
 
 /* Sort of a hack... */
 #define EOL (EOF - 1)
@@ -40,7 +49,7 @@
    as the file name for which the error was encountered.
    Then return to command level.  */
 
-void
+static void
 perror_with_name (char *string)
 {
 #ifndef STDC_HEADERS
@@ -71,7 +80,7 @@
   exit (1);
 }
 
-void
+static void
 remote_close (void)
 {
   close (remote_desc);
@@ -80,10 +89,12 @@
 /* Open a connection to a remote debugger.
    NAME is the filename used for communication.  */
 
-void
+static void
 remote_open (char *name)
 {
+#ifndef HAVE_STRING_H
   extern char *strchr ();
+#endif
 
   if (!strchr (name, ':'))
     {
@@ -230,7 +241,7 @@
 /* Accept input from gdb and match with chars from fp (after skipping one
    blank) up until a \n is read from fp (which is not matched) */
 
-void
+static void
 expect (FILE *fp)
 {
   int fromlog;
@@ -261,7 +272,7 @@
 /* Play data back to gdb from fp (after skipping leading blank) up until a
    \n is read from fp (which is discarded and not sent to gdb). */
 
-void
+static void
 play (FILE *fp)
 {
   int fromlog;

Re: [5.2.1] quiet warnings for gdbreplay.c

[Andrew Cagney]

> +#ifdef HAVE_STRING_H
> +#include <string.h>
> +#endif

Should that bit be "gdb_string.h"?

Andrew

Re: [5.2.1] quiet warnings for gdbreplay.c

[Daniel Jacobowitz]

On Fri, Jun 28, 2002 at 12:12:31PM -0400, Andrew Cagney wrote:
> >+#ifdef HAVE_STRING_H
> >+#include <string.h>
> >+#endif
> 
> Should that bit be "gdb_string.h"?

That depends :)  gdbreplay is not part of GDB and has a different
config.h header (tests for a different set of macros).  I had
eliminated all references to the gdb source code but then I introduced
an include of "gdb_proc_service.h", since the alternative was just
duplicating it; I have the feeling we should move that and headers like
gdb_string.h somewhere common - are they include/gdb/ candidates?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [5.2.1] quiet warnings for gdbreplay.c

[David O'Brien]

On Fri, Jun 28, 2002 at 02:08:20PM -0400, Daniel Jacobowitz wrote:
> On Fri, Jun 28, 2002 at 12:12:31PM -0400, Andrew Cagney wrote:
> > >+#ifdef HAVE_STRING_H
> > >+#include <string.h>
> > >+#endif
> > 
> > Should that bit be "gdb_string.h"?
> 
> That depends :)  gdbreplay is not part of GDB and has a different
> config.h header (tests for a different set of macros).  I had
> eliminated all references to the gdb source code but...

Let me know which way is best.

Re: [5.2.1] quiet warnings for gdbreplay.c

[Andrew Cagney]

> On Fri, Jun 28, 2002 at 12:12:31PM -0400, Andrew Cagney wrote:
> 
>> >+#ifdef HAVE_STRING_H
>> >+#include <string.h>
>> >+#endif
> 
>> 
>> Should that bit be "gdb_string.h"?
> 
> 
> That depends :)  gdbreplay is not part of GDB and has a different
> config.h header (tests for a different set of macros).

(We could always fix that - eliminate that configure file :-^ )

More seriously, I think someone hacking on gdbreplay is likely to also 
be hacking on GDB.  Consequently, they are going to expect the two 
directories to follow the same coding conventions.

   I had
> eliminated all references to the gdb source code but then I introduced
> an include of "gdb_proc_service.h", since the alternative was just
> duplicating it; I have the feeling we should move that and headers like
> gdb_string.h somewhere common - are they include/gdb/ candidates?

You mean #include "gdb/gdb_string.h"?  I think include/gdb/ is for 
external interfaces that are at some level controlled by GDB.

Andrew

Re: [5.2.1] quiet warnings for gdbreplay.c

[Daniel Jacobowitz]

On Fri, Jun 28, 2002 at 05:20:55PM -0400, Andrew Cagney wrote:
> >On Fri, Jun 28, 2002 at 12:12:31PM -0400, Andrew Cagney wrote:
> >
> >>>+#ifdef HAVE_STRING_H
> >>>+#include <string.h>
> >>>+#endif
> >
> >>
> >>Should that bit be "gdb_string.h"?
> >
> >
> >That depends :)  gdbreplay is not part of GDB and has a different
> >config.h header (tests for a different set of macros).
> 
> (We could always fix that - eliminate that configure file :-^ )

I dunno, I appreciate being able to run configure in the gdbserver/
directory.  It means you don't need to a terminal library for your
target if you just want gdbserver.  A lot of people have seemed to
appreciate this.

> More seriously, I think someone hacking on gdbreplay is likely to also 
> be hacking on GDB.  Consequently, they are going to expect the two 
> directories to follow the same coding conventions.

We discussed this.  The two directories share no code except for
signals/signals.c off in its own space.  If they share headers, some
care is called for.  They are not part of the same program!

>   I had
> >eliminated all references to the gdb source code but then I introduced
> >an include of "gdb_proc_service.h", since the alternative was just
> >duplicating it; I have the feeling we should move that and headers like
> >gdb_string.h somewhere common - are they include/gdb/ candidates?
> 
> You mean #include "gdb/gdb_string.h"?  I think include/gdb/ is for 
> external interfaces that are at some level controlled by GDB.

Then do we want a separate gdb/gdbint/ directory for this?  I strongly
prefer that headers shared between GDB and other directories be clearly
marked and separated.  That'd give me a place to move
gdb_proc_service.h, too.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [5.2.1] quiet warnings for gdbreplay.c

[Andrew Cagney]

> I dunno, I appreciate being able to run configure in the gdbserver/
> directory.  It means you don't need to a terminal library for your
> target if you just want gdbserver.  A lot of people have seemed to
> appreciate this.

Ah, ok, I see your problem.  Yes that does make things easier. 
GDBserver on its own is far more likely to configure/build and is easier 
to drag around.

Suggest a comment (I've no idea where) explaining this (or even mention 
it in GDB's doco - something like stating that gdbserver can be 
built/configured separate to GDB).

>> More seriously, I think someone hacking on gdbreplay is likely to also 
>> be hacking on GDB.  Consequently, they are going to expect the two 
>> directories to follow the same coding conventions.
> 
> 
> We discussed this.  The two directories share no code except for
> signals/signals.c off in its own space.  If they share headers, some
> care is called for.  They are not part of the same program!

True, but they are part of the same ``system''.

>>   I had
> 
>> >eliminated all references to the gdb source code but then I introduced
>> >an include of "gdb_proc_service.h", since the alternative was just
>> >duplicating it; I have the feeling we should move that and headers like
>> >gdb_string.h somewhere common - are they include/gdb/ candidates?
> 
>> 
>> You mean #include "gdb/gdb_string.h"?  I think include/gdb/ is for 
>> external interfaces that are at some level controlled by GDB.
> 
> 
> Then do we want a separate gdb/gdbint/ directory for this?  I strongly
> prefer that headers shared between GDB and other directories be clearly
> marked and separated.  That'd give me a place to move
> gdb_proc_service.h, too.

I think it would be easier to just clarify the guidelines for 
``gdb_XXXX.h'' files - that they be independant as they are included by 
GDB and friends.

In the mean time, I guess the status quo remains.

Andrew

Re: [5.2.1] quiet warnings for gdbreplay.c

[Daniel Jacobowitz]

On Tue, Jul 09, 2002 at 05:13:54PM -0400, Andrew Cagney wrote:
> >Then do we want a separate gdb/gdbint/ directory for this?  I strongly
> >prefer that headers shared between GDB and other directories be clearly
> >marked and separated.  That'd give me a place to move
> >gdb_proc_service.h, too.
> 
> I think it would be easier to just clarify the guidelines for 
> ``gdb_XXXX.h'' files - that they be independant as they are included by 
> GDB and friends.

This has been on my TODO list for almost a year... Andrew, how's this?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-06-21  Daniel Jacobowitz  <drow@mvista.com>

	* gdbint.texinfo (Coding): Clarify use of gdb_XXX.h headers.

Index: gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.154
diff -u -p -r1.154 gdbint.texinfo
--- gdbint.texinfo	19 Jun 2003 15:04:05 -0000	1.154
+++ gdbint.texinfo	21 Jun 2003 17:38:47 -0000
@@ -5191,7 +5191,11 @@ of @value{GDBN}) must be added to @file{
 
 When @value{GDBN} has a local version of a system header file (ex
 @file{string.h}) the file name based on the POSIX header prefixed with
-@file{gdb_} (@file{gdb_string.h}).
+@file{gdb_} (@file{gdb_string.h}).  These headers should be relatively
+independent: they should use only macros defined by @file{configure},
+the compiler, or the host; they should include only system headers; they
+should refer only to system types.  They may be shared between multiple
+programs, e.g.@: @value{GDBN} and @sc{gdbserver}.
 
 For other files @samp{-} is used as the separator.
 

Re: [5.2.1] quiet warnings for gdbreplay.c

[Andrew Cagney]

> On Tue, Jul 09, 2002 at 05:13:54PM -0400, Andrew Cagney wrote:
> 
>> >Then do we want a separate gdb/gdbint/ directory for this?  I strongly
>> >prefer that headers shared between GDB and other directories be clearly
>> >marked and separated.  That'd give me a place to move
>> >gdb_proc_service.h, too.
> 
>> 
>> I think it would be easier to just clarify the guidelines for 
>> ``gdb_XXXX.h'' files - that they be independant as they are included by 
>> GDB and friends.

Yea, ask eli about your @info.

Andrew

Re: [5.2.1] quiet warnings for gdbreplay.c

[Daniel Jacobowitz]

On Sat, Jun 21, 2003 at 01:40:02PM -0400, Daniel Jacobowitz wrote:
> On Tue, Jul 09, 2002 at 05:13:54PM -0400, Andrew Cagney wrote:
> > >Then do we want a separate gdb/gdbint/ directory for this?  I strongly
> > >prefer that headers shared between GDB and other directories be clearly
> > >marked and separated.  That'd give me a place to move
> > >gdb_proc_service.h, too.
> > 
> > I think it would be easier to just clarify the guidelines for 
> > ``gdb_XXXX.h'' files - that they be independant as they are included by 
> > GDB and friends.
> 
> This has been on my TODO list for almost a year... Andrew, how's this?

Hi Eli,

Since it seems I captured Andrew's and my intent correctly, is this
documentation patch OK?

> 2003-06-21  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* gdbint.texinfo (Coding): Clarify use of gdb_XXX.h headers.
> 
> Index: gdbint.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
> retrieving revision 1.154
> diff -u -p -r1.154 gdbint.texinfo
> --- gdbint.texinfo	19 Jun 2003 15:04:05 -0000	1.154
> +++ gdbint.texinfo	21 Jun 2003 17:38:47 -0000
> @@ -5191,7 +5191,11 @@ of @value{GDBN}) must be added to @file{
>  
>  When @value{GDBN} has a local version of a system header file (ex
>  @file{string.h}) the file name based on the POSIX header prefixed with
> -@file{gdb_} (@file{gdb_string.h}).
> +@file{gdb_} (@file{gdb_string.h}).  These headers should be relatively
> +independent: they should use only macros defined by @file{configure},
> +the compiler, or the host; they should include only system headers; they
> +should refer only to system types.  They may be shared between multiple
> +programs, e.g.@: @value{GDBN} and @sc{gdbserver}.
>  
>  For other files @samp{-} is used as the separator.
>  
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [5.2.1] quiet warnings for gdbreplay.c

[Eli Zaretskii]

> Date: Sat, 21 Jun 2003 14:07:38 -0400
> From: Daniel Jacobowitz <drow@mvista.com>
> 
> Since it seems I captured Andrew's and my intent correctly, is this
> documentation patch OK?

Yes, thanks.