[RFA] fix gdb.base/remote.c for small int targets

[RFA] fix gdb.base/remote.c for small int targets

[Jeff Holcomb]

Ok.  Hows this patch?

2001-08-30  Jeff Holcomb  <jeffh@redhat.com>

	* gdb.base/remote.c: Use a small buffer for targets with 16-bit
	ints.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.c,v
retrieving revision 1.2
diff -u -p -r1.2 remote.c
--- remote.c    2001/07/17 21:47:19     1.2
+++ remote.c    2001/08/31 05:05:33
@@ -23,16 +23,16 @@ BEGIN {
 
 */
 
-#ifdef mc68hc11
+#include <limits.h>
+/* For targets with 16bit int, use a 1k buffer. */
+#if INT_MAX < 32768
 # define RANDOM_DATA_SIZE (1024)
-#endif
-
+#else
 /* Use a character buffer to avoid byte order problems. 48k is
    chosen so that the buffer required at least 3 16k packets but
    targets often have no more than 64k of data. */
 /* If you change this data, you will also have to change the checks
    for the data in remote.c */
-#ifndef RANDOM_DATA_SIZE
 # define RANDOM_DATA_SIZE (3 * 2048 * 8)
 # define BIG_RANDOM_DATA
 #endif



--
Jeff Holcomb
jeffh@redhat.com
GDB Engineering
Red Hat, Inc.

Re: [RFA] fix gdb.base/remote.c for small int targets

[Eli Zaretskii]

> Date: Thu, 30 Aug 2001 22:11:23 -0700 (PDT)
> From: Jeff Holcomb <jeffh@redhat.com>
>  
> -#ifdef mc68hc11
> +#include <limits.h>
> +/* For targets with 16bit int, use a 1k buffer. */
> +#if INT_MAX < 32768

Shouldn't you use 32768L or 32768U?  If an int is only 16 bits wide,
32768 might overflow into the sign bit, and then all bets are off.

Re: [RFA] fix gdb.base/remote.c for small int targets

[Kevin Buettner]

On Aug 31, 10:25am, Eli Zaretskii wrote:

> > +#if INT_MAX < 32768
> 
> Shouldn't you use 32768L or 32768U?  If an int is only 16 bits wide,
> 32768 might overflow into the sign bit, and then all bets are off.

Although not incorrect, using 32768L or 32768U is not necessary. 
According to section 7.11.1 of Harbison and Steele:

    Preprocessor constant expressions must be evaluated at compile
    time, and are subject to some relatively strict constraints.  Such
    expressions must have integral type, and can involve only integer
    constants, character constants, and the special defined operator. 
    All arithmetic is to be done using types long or unsigned long, as
    appropriate to the signedness of the operands, which frees the
    preprocessor from having to deal with several sizes of integer
    types.

Kevin

Re: [RFA] fix gdb.base/remote.c for small int targets

[Eli Zaretskii]

> Date: Fri, 31 Aug 2001 08:29:02 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
> 
> > > +#if INT_MAX < 32768
> > 
> > Shouldn't you use 32768L or 32768U?  If an int is only 16 bits wide,
> > 32768 might overflow into the sign bit, and then all bets are off.
> 
> Although not incorrect, using 32768L or 32768U is not necessary. 
> According to section 7.11.1 of Harbison and Steele:

What version of the C standard is this from?  If that's C99, I don't
think we can assume all compilers comply with it.

Anyway, I know at least one compiler which would print a warning about
large constants being converted to unsigned.  I think it's best to
avoid warnings, even if they are not mandated by the standard.

Re: [RFA] fix gdb.base/remote.c for small int targets

[Kevin Buettner]

On Aug 31,  6:59pm, Eli Zaretskii wrote:

> > > > +#if INT_MAX < 32768
> > > 
> > > Shouldn't you use 32768L or 32768U?  If an int is only 16 bits wide,
> > > 32768 might overflow into the sign bit, and then all bets are off.
> > 
> > Although not incorrect, using 32768L or 32768U is not necessary. 
> > According to section 7.11.1 of Harbison and Steele:
> 
> What version of the C standard is this from?

The book that I was quoting is the Fourth Edition of "C, a Reference
Manual" by Samuel P. Harbison and Guy L. Steele Jr.  Its copyright
is 1995 by Prentice Hall.

I've reviewed the introductory material and it claims to document
ISO C.  It also documents traditional (aka K&R) C, but I believe
the book notes those places where traditional C differs from ISO C.
Also, the book documents the 1994 (Amendment 1) extensions to the
ISO C standard, but it designates these as extensions when described
in the book.

I've reread section 7.11.1 and it looks to me like it documents ISO C
(and probably traditional C too).

> If that's C99, I don't
> think we can assume all compilers comply with it.

I don't think the book that I have documents any of the C99 extensions
at all.

> Anyway, I know at least one compiler which would print a warning about
> large constants being converted to unsigned.

Could you test this again?  Remember that the expression in question is
being evaluated by the C preprocessor, not the C compiler proper.

> I think it's best to
> avoid warnings, even if they are not mandated by the standard.

I don't entirely agree; some compilers produce warnings for perfectly
reasonable code.  To avoid these warnings, the author of the code
sometimes needs to employ artifices which render the code less
intelligible to the human reader.  For the testsuite, however, it is
best to avoid *any* warnings since the testsuite harness often views
these as compilation failures and will refuse to run the test
otherwise.

Kevin

Re: [RFA] fix gdb.base/remote.c for small int targets

[Eli Zaretskii]

> Date: Fri, 31 Aug 2001 09:45:14 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
> 
> > Anyway, I know at least one compiler which would print a warning about
> > large constants being converted to unsigned.
> 
> Could you test this again?  Remember that the expression in question is
> being evaluated by the C preprocessor, not the C compiler proper.
> 
> > I think it's best to
> > avoid warnings, even if they are not mandated by the standard.
> 
> I don't entirely agree; some compilers produce warnings for perfectly
> reasonable code.

Kevin, I don't want to make a prolonged argument out of this.  If
you-all are happy with including that code verbatim, I don't mind,
either.

Re: [RFA] fix gdb.base/remote.c for small int targets

[Kevin Buettner]

On Aug 30, 10:11pm, Jeff Holcomb wrote:

> Ok.  Hows this patch?
> 
> 2001-08-30  Jeff Holcomb  <jeffh@redhat.com>
> 
> 	* gdb.base/remote.c: Use a small buffer for targets with 16-bit
> 	ints.

It looks good to me.  I recommend that this one be approved...

Kevin

[PATCH] fix gdb.base/remote.c for small int targets

[Jeff Holcomb]

Committed.

On Thu, 30 Aug 2001, Jeff Holcomb wrote:

> 2001-08-30  Jeff Holcomb  <jeffh@redhat.com>
> 
> 	* gdb.base/remote.c: Use a small buffer for targets with 16-bit
> 	ints.
> 
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 remote.c
> --- remote.c    2001/07/17 21:47:19     1.2
> +++ remote.c    2001/08/31 05:05:33
> @@ -23,16 +23,16 @@ BEGIN {
>  
>  */
>  
> -#ifdef mc68hc11
> +#include <limits.h>
> +/* For targets with 16bit int, use a 1k buffer. */
> +#if INT_MAX < 32768
>  # define RANDOM_DATA_SIZE (1024)
> -#endif
> -
> +#else
>  /* Use a character buffer to avoid byte order problems. 48k is
>     chosen so that the buffer required at least 3 16k packets but
>     targets often have no more than 64k of data. */
>  /* If you change this data, you will also have to change the checks
>     for the data in remote.c */
> -#ifndef RANDOM_DATA_SIZE
>  # define RANDOM_DATA_SIZE (3 * 2048 * 8)
>  # define BIG_RANDOM_DATA
>  #endif