[commit] Use bfd_byte in value.h

[commit] Use bfd_byte in value.h

[Andrew Cagney]

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

This changes value.h to use bfd_byte more consistently.  When trying to 
fix the signed (string) VS unsigned (buffer) pointer warnings you very 
quickly run into these interfaces.

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 4977 bytes --]

2005-04-28  Andrew Cagney  <cagney@gnu.org>

	* valops.c (check_field): Make buffer a bfd_byte.
	* value.c (unpack_long, unpack_double, unpack_pointer) 
	(unpack_field_as_long, modify_field): Ditto.
	* value.h: Update declarations.

Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.155
diff -p -u -r1.155 valops.c
--- valops.c	24 Feb 2005 13:51:35 -0000	1.155
+++ valops.c	28 Apr 2005 16:13:15 -0000
@@ -2317,7 +2317,7 @@ check_field_in (struct type *type, const
    target structure/union is defined, otherwise, return 0.  */
 
 int
-check_field (struct value *arg1, const char *name)
+check_field (struct value *arg1, const bfd_byte *name)
 {
   struct type *t;
 
Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.27
diff -p -u -r1.27 value.c
--- value.c	20 Feb 2005 01:19:24 -0000	1.27
+++ value.c	28 Apr 2005 16:13:16 -0000
@@ -996,7 +996,7 @@ value_as_address (struct value *val)
    to an INT (or some size).  After all, it is only an offset.  */
 
 LONGEST
-unpack_long (struct type *type, const char *valaddr)
+unpack_long (struct type *type, const bfd_byte *valaddr)
 {
   enum type_code code = TYPE_CODE (type);
   int len = TYPE_LENGTH (type);
@@ -1045,7 +1045,7 @@ unpack_long (struct type *type, const ch
    format, result is in host format.  */
 
 DOUBLEST
-unpack_double (struct type *type, const char *valaddr, int *invp)
+unpack_double (struct type *type, const bfd_byte *valaddr, int *invp)
 {
   enum type_code code;
   int len;
@@ -1109,7 +1109,7 @@ unpack_double (struct type *type, const 
    to an INT (or some size).  After all, it is only an offset.  */
 
 CORE_ADDR
-unpack_pointer (struct type *type, const char *valaddr)
+unpack_pointer (struct type *type, const bfd_byte *valaddr)
 {
   /* Assume a CORE_ADDR can fit in a LONGEST (for now).  Not sure
      whether we want this to be true eventually.  */
@@ -1359,7 +1359,7 @@ value_fn_field (struct value **arg1p, st
    If the field is signed, we also do sign extension. */
 
 LONGEST
-unpack_field_as_long (struct type *type, const char *valaddr, int fieldno)
+unpack_field_as_long (struct type *type, const bfd_byte *valaddr, int fieldno)
 {
   ULONGEST val;
   ULONGEST valmask;
@@ -1406,7 +1406,7 @@ unpack_field_as_long (struct type *type,
    0 <= BITPOS, where lbits is the size of a LONGEST in bits.  */
 
 void
-modify_field (char *addr, LONGEST fieldval, int bitpos, int bitsize)
+modify_field (bfd_byte *addr, LONGEST fieldval, int bitpos, int bitsize)
 {
   ULONGEST oword;
   ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.85
diff -p -u -r1.85 value.h
--- value.h	8 Mar 2005 14:35:17 -0000	1.85
+++ value.h	28 Apr 2005 16:13:16 -0000
@@ -262,11 +262,12 @@ extern LONGEST value_as_long (struct val
 extern DOUBLEST value_as_double (struct value *val);
 extern CORE_ADDR value_as_address (struct value *val);
 
-extern LONGEST unpack_long (struct type *type, const char *valaddr);
-extern DOUBLEST unpack_double (struct type *type, const char *valaddr,
+extern LONGEST unpack_long (struct type *type, const bfd_byte *valaddr);
+extern DOUBLEST unpack_double (struct type *type, const bfd_byte *valaddr,
 			       int *invp);
-extern CORE_ADDR unpack_pointer (struct type *type, const char *valaddr);
-extern LONGEST unpack_field_as_long (struct type *type, const char *valaddr,
+extern CORE_ADDR unpack_pointer (struct type *type, const bfd_byte *valaddr);
+extern LONGEST unpack_field_as_long (struct type *type,
+				     const bfd_byte *valaddr,
 				     int fieldno);
 
 extern struct value *value_from_longest (struct type *type, LONGEST num);
@@ -451,14 +452,15 @@ extern void release_value (struct value 
 
 extern int record_latest_value (struct value *val);
 
-extern void modify_field (char *addr, LONGEST fieldval, int bitpos,
+extern void modify_field (bfd_byte *addr, LONGEST fieldval, int bitpos,
 			  int bitsize);
 
 extern void type_print (struct type *type, char *varstring,
 			struct ui_file *stream, int show);
 
-extern char *baseclass_addr (struct type *type, int index, char *valaddr,
-			     struct value **valuep, int *errp);
+extern bfd_byte *baseclass_addr (struct type *type, int index,
+				 bfd_byte *valaddr,
+				 struct value **valuep, int *errp);
 
 extern void print_longest (struct ui_file *stream, int format,
 			   int use_local, LONGEST val);
@@ -493,7 +495,7 @@ extern void print_variable_value (struct
 				  struct frame_info *frame,
 				  struct ui_file *stream);
 
-extern int check_field (struct value *, const char *);
+extern int check_field (struct value *, const bfd_byte *);
 
 extern void typedef_print (struct type *type, struct symbol *news,
 			   struct ui_file *stream);

Re: [commit] Use bfd_byte in value.h

[Mark Kettenis]

   Date: Thu, 28 Apr 2005 12:25:52 -0400
   From: Andrew Cagney <cagney@gnu.org>

   This changes value.h to use bfd_byte more consistently.  When trying to 
   fix the signed (string) VS unsigned (buffer) pointer warnings you very 
   quickly run into these interfaces.

Hacking on a mac again?

Anyway, to place my previous comments about this issue in a context,
this really is analogous to strxxx() versus memxxx().  This gives a
clue to what the proper pointer type for these buffers is: `void *'.
I guess GDB's K&R history really shows here.  Does converting to `void
*' require more effort than simply replacing things with `bfd_byte *'?

Mark

Re: [commit] Use bfd_byte in value.h

[Daniel Jacobowitz]

On Thu, Apr 28, 2005 at 09:19:20PM +0200, Mark Kettenis wrote:
>    Date: Thu, 28 Apr 2005 12:25:52 -0400
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    This changes value.h to use bfd_byte more consistently.  When trying to 
>    fix the signed (string) VS unsigned (buffer) pointer warnings you very 
>    quickly run into these interfaces.
> 
> Hacking on a mac again?
> 
> Anyway, to place my previous comments about this issue in a context,
> this really is analogous to strxxx() versus memxxx().  This gives a
> clue to what the proper pointer type for these buffers is: `void *'.
> I guess GDB's K&R history really shows here.  Does converting to `void
> *' require more effort than simply replacing things with `bfd_byte *'?

Probably, since you can no longer do arithmetic on them.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [commit] Use bfd_byte in value.h

[Andrew Cagney]

Like I mentioned on gdb@, these are for warnings you get when building 
with GCC 4 (or for that matter, any modern compiler).  vis:

cagney@tornado$ cat -n s.c
      1  char *c;
      2  signed char *s;
      3  unsigned char *u;
      4
      5  void foo (void)
      6  {
      7    c = s;
      8    c = u;
      9    s = u;
     10  }
$ gcc --version
gcc (GCC) 4.0.0 20050412 (Red Hat 4.0.0-0.42)
...
$ gcc -c s.c
s.c: In function foo:
s.c:7: warning: pointer targets in assignment differ in signedness
s.c:8: warning: pointer targets in assignment differ in signedness
s.c:9: warning: pointer targets in assignment differ in signedness

Re: [commit] Use bfd_byte in value.h

[Eli Zaretskii]

> Date: Thu, 28 Apr 2005 18:08:40 -0400
> From: Andrew Cagney <cagney@gnu.org>
> CC: gdb-patches@sources.redhat.com
> 
> Like I mentioned on gdb@, these are for warnings you get when building 
> with GCC 4 (or for that matter, any modern compiler).  vis:
> 
> cagney@tornado$ cat -n s.c
>       1  char *c;
>       2  signed char *s;
>       3  unsigned char *u;
>       4
>       5  void foo (void)
>       6  {
>       7    c = s;
>       8    c = u;
>       9    s = u;
>      10  }
> $ gcc --version
> gcc (GCC) 4.0.0 20050412 (Red Hat 4.0.0-0.42)
> ...
> $ gcc -c s.c
> s.c: In function foo:
> s.c:7: warning: pointer targets in assignment differ in signedness
> s.c:8: warning: pointer targets in assignment differ in signedness
> s.c:9: warning: pointer targets in assignment differ in signedness

But why should we solve this with bfd_byte?  Why not introduce GDB's
own data type, like gdb_byte or some such?

BFD is just a library; there's no need to pollute our name space with
BFD's.  It's IMHO unclean.

But I already said this, and was already ignored when I did, sigh...

Re: [commit] Use bfd_byte in value.h

[Andrew Cagney]

Eli,

Last time this came up I recommeded separating out the idea of gdb_byte 
and proposed more formally.  How is this going?

In the mean time though (then and now) I'll use bfd_byte as that lets me 
move this code forward addressing the more immediate problem of not even 
compiling with -Werror on GCC 4.  With that done, i'm pretty sure that 
we'll find the follow-on task of s/bfd_byte/gdb_byte/ trivial.

Re: [commit] Use bfd_byte in value.h

[Mark Kettenis]

   Date: Sun, 01 May 2005 16:17:28 -0400
   From: Andrew Cagney <cagney@gnu.org>

   Eli,

   Last time this came up I recommeded separating out the idea of gdb_byte 
   and proposed more formally.  How is this going?

   In the mean time though (then and now) I'll use bfd_byte as that lets me 
   move this code forward addressing the more immediate problem of not even 
   compiling with -Werror on GCC 4.  With that done, i'm pretty sure that 
   we'll find the follow-on task of s/bfd_byte/gdb_byte/ trivial.

But why do we need special type at all.  ISO C has void * for
arbitrary (byte) buffers.  That's the type that should have been used
for the memory_insert_breakpoint/memory_remove_breakpoint interface
changes you just committed.  Any implementation of that interface
should do the appropriate cast if it needs to do pointer arithmetic,
or needs to interpret the bytes as signed or unsigned.  If we agree on
that, this makes the follow-in task you mention non-trivial.  Since
you are alreading spending an effort on this, why not spend a small
amount of extra effort and fix things properly now?

Mark

Re: [commit] Use bfd_byte in value.h

[Eli Zaretskii]

> Date: Sun, 01 May 2005 16:17:28 -0400
> From: Andrew Cagney <cagney@gnu.org>
> CC: mark.kettenis@xs4all.nl, gdb-patches@sources.redhat.com
> 
> Eli,
> 
> Last time this came up I recommeded separating out the idea of gdb_byte 
> and proposed more formally.  How is this going?

Sorry, I'm not sure I understand (or maybe I forgot).  Can you please
point me to the message with your proposal?

> In the mean time though (then and now) I'll use bfd_byte as that lets me 
> move this code forward addressing the more immediate problem of not even 
> compiling with -Werror on GCC 4.  With that done, i'm pretty sure that 
> we'll find the follow-on task of s/bfd_byte/gdb_byte/ trivial.

I'd rather discuss first and decide, then implement.  This way, you
won't need to s/bfd_byte/gdb_byte/, however trivial.  The discussion
should not last long enough to delay the solution for GCC 4: after
all, it seems like a very simple issue.  With any luck, we could agree
on something in a day or two.

Re: [commit] Use bfd_byte in value.h

[Andrew Cagney]

Eli Zaretskii wrote:
>>Date: Sun, 01 May 2005 16:17:28 -0400
>>From: Andrew Cagney <cagney@gnu.org>
>>CC: mark.kettenis@xs4all.nl, gdb-patches@sources.redhat.com
>>
>>Eli,
>>
>>Last time this came up I recommeded separating out the idea of gdb_byte 
>>and proposed more formally.  How is this going?
> 
> 
> Sorry, I'm not sure I understand (or maybe I forgot).  Can you please
> point me to the message with your proposal?

The suggestion of gdb_byte was yours, so it rightfully falls to you to 
propose more formally on gdb@ (I certaintly don't want to jump in and 
steal your thunder).  Can we finally do that?

BTW, just like with value.[hc], I've been careful to only make the 
straight forward change for buffers of s/{unsigned,}char/bfd_byte/, and 
have tried to avoid more contraversial replacements such as 
s/void*/bfd_byte*/.  We're hardly going to revert the existing bfd_byte 
changes.

Re: [commit] Use bfd_byte in value.h

[Eli Zaretskii]

> Date: Sun, 01 May 2005 19:01:40 -0400
> From: Andrew Cagney <cagney@gnu.org>
> CC: mark.kettenis@xs4all.nl, gdb-patches@sources.redhat.com
> 
> The suggestion of gdb_byte was yours, so it rightfully falls to you to 
> propose more formally on gdb@ (I certaintly don't want to jump in and 
> steal your thunder).  Can we finally do that?

I will happily do whatever it takes, but I'm unsure what you want me
to say there.  That I propose to replace "char *" with "gdb_byte *"?
Somehow, I doubt that this is all you read into ``propose formally''.
But if all you are asking is to start a thread on gdb@, I'll do this.

In any case, I think Mark's question (why not use "void *") is
something that we should discuss first, since if we accept his
recommendation, there will be no need to introduce a new data type.

Re: [commit] Use bfd_byte in value.h

[Andrew Cagney]

... and such debate belongs on gdb@.  Presumably Mark will respond to 
your proposal.


Eli Zaretskii wrote:
>>Date: Sun, 01 May 2005 19:01:40 -0400
>>From: Andrew Cagney <cagney@gnu.org>
>>CC: mark.kettenis@xs4all.nl, gdb-patches@sources.redhat.com
>>
>>The suggestion of gdb_byte was yours, so it rightfully falls to you to 
>>propose more formally on gdb@ (I certaintly don't want to jump in and 
>>steal your thunder).  Can we finally do that?
> 
> 
> I will happily do whatever it takes, but I'm unsure what you want me
> to say there.  That I propose to replace "char *" with "gdb_byte *"?
> Somehow, I doubt that this is all you read into ``propose formally''.
> But if all you are asking is to start a thread on gdb@, I'll do this.
> 
> In any case, I think Mark's question (why not use "void *") is
> something that we should discuss first, since if we accept his
> recommendation, there will be no need to introduce a new data type.
> 

Re: [commit] Use bfd_byte in value.h

[Eli Zaretskii]

> Date: Tue, 03 May 2005 10:42:46 -0400
> From: Andrew Cagney <cagney@gnu.org>
> CC: mark.kettenis@xs4all.nl, gdb-patches@sources.redhat.com
> 
> ... and such debate belongs on gdb@.

Done.

Now I hope that you will not continue committing bfd_byte changes
until that discussion ends in some kind of agreement.

Re: [commit] Use bfd_byte in value.h

[Daniel Jacobowitz]

On Sun, May 01, 2005 at 07:01:40PM -0400, Andrew Cagney wrote:
> The suggestion of gdb_byte was yours, so it rightfully falls to you to 
> propose more formally on gdb@ (I certaintly don't want to jump in and 
> steal your thunder).  Can we finally do that?
> 
> BTW, just like with value.[hc], I've been careful to only make the 
> straight forward change for buffers of s/{unsigned,}char/bfd_byte/, and 
> have tried to avoid more contraversial replacements such as 
> s/void*/bfd_byte*/.  We're hardly going to revert the existing bfd_byte 
> changes.

Despite the fact that Mark _asked_ you to revert your last patch (which
you've ignored)?

-- 
Daniel Jacobowitz
CodeSourcery, LLC