* A case for `void *' for pointers to arbitrary (byte) buffers [not found] ` <42778DE6.1080106@gnu.org> @ 2005-05-03 20:17 ` Mark Kettenis 2005-05-03 20:23 ` Daniel Jacobowitz 2005-05-03 22:01 ` Stan Shebs 0 siblings, 2 replies; 12+ messages in thread From: Mark Kettenis @ 2005-05-03 20:17 UTC (permalink / raw) To: gdb; +Cc: cagney, eliz Date: Tue, 03 May 2005 10:42:46 -0400 From: Andrew Cagney <cagney@gnu.org> ... and such debate belongs on gdb@. Presumably Mark will respond to your proposal. OK, here it is. The Problem ----------- In many places GDB needs to pass along and pick apart blobs of memory. In many cases, GDB does this using `char *' pointers. Small blobs of memory are often allocated on the stack as `char[]' or `unsigned char[]'. Unfortunately ISO C doesn't specify whether `char' is signed or unsigned. On most systems, char is signed, but on some systems (most notably powerpc) it is unsigned. This leads to many platform-dependent type conversion bugs in code. Since we have been sloppy about the (un)signedness of char in the GDB codebase, there probably are quite a few of these bugs in GDB. Fortunately recent versions of GCC have started to warn about suspicous type conversions. But because of our sloppyness, the current GDB doesn't compiler with -Werror on systems with this new version of GCC. Compiling without -Werror will hide many bugs in our codebase, so we should fix this in GDB. The proper pointer type for arbitrary buffers --------------------------------------------- Looking at ISO C and in particular the ISO C standard library, we notice that there are two families of functions that deal with blobs of memory: * Functions starting with mem, such as memcpy(), memset(), etc. These functions all use the `void *' type to refer to the blobs of memory they operate on. * Functions starting with str, such as strcpy(), strlen(), etc. These functions don't really operate on arbitrary blobs of memory, but rather on NUL-terminated character strings. They all use the `char *' type to refer to these strings. So from a language standpoint of view we should use `char *' for NUL-terminated sequences of bytes and `void *' for arbitrary sequences of bytes. A nice thing about `void *' is that you can implicitly cast it to every other pointer type in the world. Why doesn't GDB use `void *'? ----------------------------- Well, it does, in some places. But the vast majority of our codebase predates ISO C, or at least GDB was supposed to be compiled on compilers that predate ISO C. We couldn't count on `void *' being part of the standard. And even after we made an ISO C compiler a requirement for compiling GDB new code still followed the bad examples from the K&R era. And of course there is also some laziness involved. One of the good things about `void *' pointers is that you can't do any pointer arithmetic with them. So if you want to do pointer arithmetic (and GDB has some genuine need to do that) you'll have to explicitly cast to the right pointer type. This is a good thing. This forces you to think about what you're doing. In the cases where it matters. An example ---------- An example of a good GDB interface where the above applies is extract_unsigned_integer(). Its full prototype is: LONGEST extract_unsigned_integer (const void *addr, int len); Its implementation converts the `void *' pointer into an `unsigned char *' pointer before doing pointer arithmetic. An example of a bad interface is read_memory(). Its full prototype is: void read_memory (CORE_ADDR memaddr, char *myaddr, int len) This function reads LEN bytes from MEMADDR and stores them in MYADDR. But MYADDR is not necessarily a NUL-terminated string. The second argument should be `void *'. Recently a few `struct value'-related functions were converted from using `char *' to `bfd_byte *'. If we take for example unpack_long(): LONGEST unpack_long(struct type *, const bfd_byte *valaddr); Its interface is very similar to extract_unsigned_integer(), in fact it is implemented using extract_unsigned_integer(). Therefore it should use the same pointer type as extract_unsigned_integer(): `void *'. Do we need a `bfd_byte *' or `gdb_byte *'? ------------------------------------------ We might. Many implementations will need to cast to some sort of byte type to do arithmetic or look at individual bytes. As we have seen, using can be `char *' is problematic. In most (problematic) cases we really mean `unsigned char', only a few will warrant `signed char'. And `bfd_byte *' is a nice abbreviation. Personally I'm perfectly happy with writing `unsigned char'. IMHO it has the benefit that it is perfectly clear that this type is unsigned, even to someone who has never been exposed to the GDB codebase. Why not use `xxx_byte *' instead of `void *'? --------------------------------------------- * It's nonstandard. Why do we need a nonstandard type if a perfectly god standard type is available? * It doesn't really solve the issue. It only propagates the problem one level up or down. Since `xxx_byte *' is nothing but a typedef for `unsigned char *', someone calling a functions with `xxx_byte *' as one of its arguments with a `char *' argument will suffer from the warning that raised this entire issue; `void *' breaks the chain immediately. Conclusion ---------- It's clear that we should work actively towards reserving the unqualified `char *' for strings, and probably never ever use an unqualified `char' as a data type (the C type of a single character is `int'). Since GCC 4.0 has been released and will probably be used by many in a few months time, we should solve this issue now, and preferably in our complete codebase. Hopefully we can get consesus soon, and start taking this issue into account when we review patches. Mark ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 20:17 ` A case for `void *' for pointers to arbitrary (byte) buffers Mark Kettenis @ 2005-05-03 20:23 ` Daniel Jacobowitz 2005-05-03 21:14 ` Mark Kettenis 2005-05-03 22:01 ` Stan Shebs 1 sibling, 1 reply; 12+ messages in thread From: Daniel Jacobowitz @ 2005-05-03 20:23 UTC (permalink / raw) To: Mark Kettenis; +Cc: gdb, cagney, eliz On Tue, May 03, 2005 at 10:13:01PM +0200, Mark Kettenis wrote: > Do we need a `bfd_byte *' or `gdb_byte *'? > ------------------------------------------ > > We might. Many implementations will need to cast to some sort of byte > type to do arithmetic or look at individual bytes. As we have seen, > using can be `char *' is problematic. In most (problematic) cases we > really mean `unsigned char', only a few will warrant `signed char'. > And `bfd_byte *' is a nice abbreviation. Personally I'm perfectly > happy with writing `unsigned char'. IMHO it has the benefit that it > is perfectly clear that this type is unsigned, even to someone who has > never been exposed to the GDB codebase. I think that using gdb_byte has a consistency advantage over typing "unsigned char" everywhere. We will know that we are dealing with opaque blobs of (usually) target memory, but retain the ability to do arithmetic on them. > Why not use `xxx_byte *' instead of `void *'? > --------------------------------------------- > > * It's nonstandard. Why do we need a nonstandard type if a perfectly > god standard type is available? > > * It doesn't really solve the issue. It only propagates the problem > one level up or down. Since `xxx_byte *' is nothing but a typedef > for `unsigned char *', someone calling a functions with `xxx_byte *' > as one of its arguments with a `char *' argument will suffer from > the warning that raised this entire issue; `void *' breaks the chain > immediately. I think that's a bad thing! For the same reason that we use -Werror: where possible, we can let GCC enforce consistency within our source base. Use of gdb_byte (as unsigned char) buys you the advantage that any other pointer type won't silently convert to it. If you want to use a standard type, play the necessary autoconf games to acquire stdint.h. Use uint8_t *. -- Daniel Jacobowitz CodeSourcery, LLC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 20:23 ` Daniel Jacobowitz @ 2005-05-03 21:14 ` Mark Kettenis 2005-05-03 21:16 ` Daniel Jacobowitz 2005-05-04 15:40 ` Andrew Cagney 0 siblings, 2 replies; 12+ messages in thread From: Mark Kettenis @ 2005-05-03 21:14 UTC (permalink / raw) To: drow; +Cc: gdb, cagney, eliz Date: Tue, 3 May 2005 16:23:52 -0400 From: Daniel Jacobowitz <drow@false.org> > Why not use `xxx_byte *' instead of `void *'? > --------------------------------------------- > > * It's nonstandard. Why do we need a nonstandard type if a perfectly > god standard type is available? > > * It doesn't really solve the issue. It only propagates the problem > one level up or down. Since `xxx_byte *' is nothing but a typedef > for `unsigned char *', someone calling a functions with `xxx_byte *' > as one of its arguments with a `char *' argument will suffer from > the warning that raised this entire issue; `void *' breaks the chain > immediately. I think that's a bad thing! For the same reason that we use -Werror: where possible, we can let GCC enforce consistency within our source base. Use of gdb_byte (as unsigned char) buys you the advantage that any other pointer type won't silently convert to it. Ah, but these are supposed to be opaque blobs of memory. If you want to use a standard type, play the necessary autoconf games to acquire stdint.h. Use uint8_t *. That's an interesting suggestion. It might take a few iterations to get that right though. Mark ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 21:14 ` Mark Kettenis @ 2005-05-03 21:16 ` Daniel Jacobowitz 2005-05-03 22:10 ` Mark Kettenis 2005-05-04 15:40 ` Andrew Cagney 1 sibling, 1 reply; 12+ messages in thread From: Daniel Jacobowitz @ 2005-05-03 21:16 UTC (permalink / raw) To: Mark Kettenis; +Cc: gdb, cagney, eliz On Tue, May 03, 2005 at 11:13:24PM +0200, Mark Kettenis wrote: > Date: Tue, 3 May 2005 16:23:52 -0400 > From: Daniel Jacobowitz <drow@false.org> > > > Why not use `xxx_byte *' instead of `void *'? > > --------------------------------------------- > > > > * It's nonstandard. Why do we need a nonstandard type if a perfectly > > god standard type is available? > > > > * It doesn't really solve the issue. It only propagates the problem > > one level up or down. Since `xxx_byte *' is nothing but a typedef > > for `unsigned char *', someone calling a functions with `xxx_byte *' > > as one of its arguments with a `char *' argument will suffer from > > the warning that raised this entire issue; `void *' breaks the chain > > immediately. > > I think that's a bad thing! For the same reason that we use -Werror: > where possible, we can let GCC enforce consistency within our source > base. Use of gdb_byte (as unsigned char) buys you the advantage that > any other pointer type won't silently convert to it. > > Ah, but these are supposed to be opaque blobs of memory. That's my point. This way we can only pass "opaque blobs of memory" to the interfaces that want opaque blobs. It's a question of categorization. For instance, this is a bug: long foo; target_read_memory (addr, &foo, 4); If the second argument is "void *", a C compiler won't complain about this. But for "uint8_t *", it will. > If you want to use a standard type, play the necessary autoconf games > to acquire stdint.h. Use uint8_t *. > > That's an interesting suggestion. It might take a few iterations to > get that right though. I'm willing to do the work. -- Daniel Jacobowitz CodeSourcery, LLC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 21:16 ` Daniel Jacobowitz @ 2005-05-03 22:10 ` Mark Kettenis 2005-05-03 22:28 ` Daniel Jacobowitz 0 siblings, 1 reply; 12+ messages in thread From: Mark Kettenis @ 2005-05-03 22:10 UTC (permalink / raw) To: drow; +Cc: gdb, cagney, eliz Date: Tue, 3 May 2005 17:16:46 -0400 From: Daniel Jacobowitz <drow@false.org> On Tue, May 03, 2005 at 11:13:24PM +0200, Mark Kettenis wrote: > Date: Tue, 3 May 2005 16:23:52 -0400 > From: Daniel Jacobowitz <drow@false.org> > > > Why not use `xxx_byte *' instead of `void *'? > > --------------------------------------------- > > > > * It's nonstandard. Why do we need a nonstandard type if a perfectly > > god standard type is available? > > > > * It doesn't really solve the issue. It only propagates the problem > > one level up or down. Since `xxx_byte *' is nothing but a typedef > > for `unsigned char *', someone calling a functions with `xxx_byte *' > > as one of its arguments with a `char *' argument will suffer from > > the warning that raised this entire issue; `void *' breaks the chain > > immediately. > > I think that's a bad thing! For the same reason that we use -Werror: > where possible, we can let GCC enforce consistency within our source > base. Use of gdb_byte (as unsigned char) buys you the advantage that > any other pointer type won't silently convert to it. > > Ah, but these are supposed to be opaque blobs of memory. That's my point. This way we can only pass "opaque blobs of memory" to the interfaces that want opaque blobs. It's a question of categorization. For instance, this is a bug: long foo; target_read_memory (addr, &foo, 4); If the second argument is "void *", a C compiler won't complain about this. But for "uint8_t *", it will. Hmm, you have a point here... > If you want to use a standard type, play the necessary autoconf games > to acquire stdint.h. Use uint8_t *. > > That's an interesting suggestion. It might take a few iterations to > get that right though. I'm willing to do the work. OK great! I really like being able to use those (u)intN_t types, and especially (u)intptr_t. You can probably steal the autoconf checks from GNU coreutils. Those will have been pretty well tested I sppose. But the base sequence should something like: #ifdef HAVE_STDINT_H #include <stdint.h> #else # ifdef HAVE_INTTYPES_H # include <inttypes.h> # else typedef unsigned char uint8_t; # endif #endif Mark ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 22:10 ` Mark Kettenis @ 2005-05-03 22:28 ` Daniel Jacobowitz 2005-05-03 22:31 ` Joel Brobecker 2005-05-04 14:45 ` Mark Kettenis 0 siblings, 2 replies; 12+ messages in thread From: Daniel Jacobowitz @ 2005-05-03 22:28 UTC (permalink / raw) To: Mark Kettenis; +Cc: gdb, cagney, eliz On Wed, May 04, 2005 at 12:06:26AM +0200, Mark Kettenis wrote: > > If you want to use a standard type, play the necessary autoconf games > > to acquire stdint.h. Use uint8_t *. > > > > That's an interesting suggestion. It might take a few iterations to > > get that right though. > > I'm willing to do the work. > > OK great! I really like being able to use those (u)intN_t types, and > especially (u)intptr_t. You can probably steal the autoconf checks > from GNU coreutils. Those will have been pretty well tested I sppose. > But the base sequence should something like: > > #ifdef HAVE_STDINT_H > #include <stdint.h> > #else > # ifdef HAVE_INTTYPES_H > # include <inttypes.h> > # else > typedef unsigned char uint8_t; > # endif > #endif To be honest, my preference is still for gdb_byte * over uint8_t *, for exactly the reason Stan just mentioned: consistency. Whether that's defined as unsigned char or uint8_t is less important to me. Does gdb_byte, defined in terms of uint8_t, work for you? -- Daniel Jacobowitz CodeSourcery, LLC ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 22:28 ` Daniel Jacobowitz @ 2005-05-03 22:31 ` Joel Brobecker 2005-05-04 14:45 ` Mark Kettenis 1 sibling, 0 replies; 12+ messages in thread From: Joel Brobecker @ 2005-05-03 22:31 UTC (permalink / raw) To: Mark Kettenis, gdb, cagney, eliz > To be honest, my preference is still for gdb_byte * over uint8_t *, for > exactly the reason Stan just mentioned: consistency. Whether that's > defined as unsigned char or uint8_t is less important to me. This is my preference too. -- Joel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 22:28 ` Daniel Jacobowitz 2005-05-03 22:31 ` Joel Brobecker @ 2005-05-04 14:45 ` Mark Kettenis 2005-05-04 20:36 ` Eli Zaretskii 1 sibling, 1 reply; 12+ messages in thread From: Mark Kettenis @ 2005-05-04 14:45 UTC (permalink / raw) To: drow; +Cc: gdb, cagney, eliz Date: Tue, 3 May 2005 18:27:44 -0400 From: Daniel Jacobowitz <drow@false.org> To be honest, my preference is still for gdb_byte * over uint8_t *, for exactly the reason Stan just mentioned: consistency. Whether that's defined as unsigned char or uint8_t is less important to me. So there seems to be a concensus for using `gdb_byte' for target byte buffers. Unless Eli has anything to add to the discussion, I'll consider this discussion closed. Does gdb_byte, defined in terms of uint8_t, work for you? This would be silly. It's not that `uint8_t' will be anything other than `unsigned char' on any system supported by gdb in the foreseeable future. Let's simply add typedef unsigned char gdb_byte; to "defs.h" and use that in any future changes. Andrew, you want to do the honour? Once that is done, people should update the targets they maintain to use it and get rid of `bfd_byte' ASAP. Mark ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-04 14:45 ` Mark Kettenis @ 2005-05-04 20:36 ` Eli Zaretskii 0 siblings, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2005-05-04 20:36 UTC (permalink / raw) To: Mark Kettenis; +Cc: drow, gdb, cagney > Date: Wed, 4 May 2005 16:39:39 +0200 (CEST) > From: Mark Kettenis <mark.kettenis@xs4all.nl> > CC: gdb@sourceware.org, cagney@gnu.org, eliz@gnu.org > > So there seems to be a concensus for using `gdb_byte' for target byte > buffers. Unless Eli has anything to add to the discussion, I'll > consider this discussion closed. I'm happy with gdb_byte; after all, it was my suggestion to begin with. I thought that your idea of void * was better, but if everyone else is for gdb_byte, let's go with that. Thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 21:14 ` Mark Kettenis 2005-05-03 21:16 ` Daniel Jacobowitz @ 2005-05-04 15:40 ` Andrew Cagney 2005-05-04 17:57 ` Eli Zaretskii 1 sibling, 1 reply; 12+ messages in thread From: Andrew Cagney @ 2005-05-04 15:40 UTC (permalink / raw) To: Mark Kettenis, drow, eliz; +Cc: gdb (yet again something ate my e-mail, I'm reading from the web) Mark described the problem as: > In many places GDB needs to pass along and pick apart blobs of memory. > In many cases, GDB does this using `char *' pointers. Small blobs of > memory are often allocated on the stack as `char[]' or `unsigned > char[]'. This is not correct. 'void *' is more applicable to arbitrary opaque objects. Here, though, we know the object's type (a byte buffer) and hence should use that. Daniel correctly explains it thus (from a disconnected thread): > These are byte-oriented buffers, so using a type where we can perform > byte-oriented arithmetic without superfluous casts seems like a good > choice to me. Converting to void * (and not using the GCC extension > which allows arithmetic on void *) would be a painful process. (And as someone that actually thought to try out the two alternatives and determine the damage, I can tell you that void* and casts totally sux.) Also, in support of this move away from void*, BINUTILS, several months ago, and in response to this rumored warning, re-vamped their types moving more strongly towards bfd_byte* (one consequence was that I had to update some of GDB's interfaces). As for the names bfd_byte vs gdb_byte that really is arbitrary. To ensure that our byte buffers are compatible with BFDs we'll want to use "typedef bfd_byte gdb_byte" anyway. Given that we're already doing similar for LONGEST, ADDRESS, et.al., that shouldn't be contraversial. Anyway, since the prefered name is "gdb_byte", and that was Eli's idea, I think it is fit and proper that Eli take the honours. Can we get it done by tomorrow? If not let me know (Hopefully I can now go back to fixing my GCC 4 warnings). Andrew ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-04 15:40 ` Andrew Cagney @ 2005-05-04 17:57 ` Eli Zaretskii 0 siblings, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2005-05-04 17:57 UTC (permalink / raw) To: cagney; +Cc: mark.kettenis, drow, gdb > Date: Wed, 04 May 2005 11:36:56 -0400 > From: Andrew Cagney <cagney@gnu.org> > > To ensure that our byte buffers are compatible with BFDs we'll want to use > "typedef bfd_byte gdb_byte" anyway. Can you explain why? I don't see why we will want that. > Anyway, since the prefered name is "gdb_byte", and that was Eli's idea, > I think it is fit and proper that Eli take the honours. Can we get it > done by tomorrow? If ``we'' here means me, then no, ``we'' cannot do this by tomorrow. I won't have enough free time until then, and on top of that, since the FSF moved to their new offices, I have trouble even reading my mail, let alone doing something more serious. Sorry. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: A case for `void *' for pointers to arbitrary (byte) buffers 2005-05-03 20:17 ` A case for `void *' for pointers to arbitrary (byte) buffers Mark Kettenis 2005-05-03 20:23 ` Daniel Jacobowitz @ 2005-05-03 22:01 ` Stan Shebs 1 sibling, 0 replies; 12+ messages in thread From: Stan Shebs @ 2005-05-03 22:01 UTC (permalink / raw) To: Mark Kettenis; +Cc: gdb, cagney, eliz Mark Kettenis wrote: >Why not use `xxx_byte *' instead of `void *'? >--------------------------------------------- > >* It's nonstandard. Why do we need a nonstandard type if a perfectly > god standard type is available? > My inclination would be for gdb_byte *, as an abstraction of "what people who understand the issue more than I do think is the right type to use". 1/2 :-) Personally I've never been comfortable with using void * because it has its own set of semi-mysterious rules for use - yeah, if I wear my compiler-guy hat, they work that way for a reason, but when I wear my just-wanna-write-code hat, they are more of a distraction. Another downside of void * is it doesn't tell the reader whether it means GDB's byte abstraction, or something else, such as a declaration that has to be void * for consistency with system headers or some such. GDB seems like a big enough program to justify its own byte type. Stan ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2005-05-04 20:36 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <42710E90.3030300@gnu.org> [not found] ` <200504281919.j3SJJKF1011501@elgar.sibelius.xs4all.nl> [not found] ` <42715EE8.5070704@gnu.org> [not found] ` <01c54c8a$Blat.v2.4$ffbe8140@zahav.net.il> [not found] ` <42753958.70109@gnu.org> [not found] ` <01c54e92$Blat.v2.4$5cf24460@zahav.net.il> [not found] ` <42755FD4.8000009@gnu.org> [not found] ` <01c54f4a$Blat.v2.4$a9fc8500@zahav.net.il> [not found] ` <42778DE6.1080106@gnu.org> 2005-05-03 20:17 ` A case for `void *' for pointers to arbitrary (byte) buffers Mark Kettenis 2005-05-03 20:23 ` Daniel Jacobowitz 2005-05-03 21:14 ` Mark Kettenis 2005-05-03 21:16 ` Daniel Jacobowitz 2005-05-03 22:10 ` Mark Kettenis 2005-05-03 22:28 ` Daniel Jacobowitz 2005-05-03 22:31 ` Joel Brobecker 2005-05-04 14:45 ` Mark Kettenis 2005-05-04 20:36 ` Eli Zaretskii 2005-05-04 15:40 ` Andrew Cagney 2005-05-04 17:57 ` Eli Zaretskii 2005-05-03 22:01 ` Stan Shebs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).