[COMMIT PATCH] target.c: fix -Wpointer-sign

[COMMIT PATCH] target.c: fix -Wpointer-sign

[Pedro Alves]

$ make WERROR_CFLAGS="-Wpointer-sign -Werror" target.o -k 2>&1 1>/dev/null
../../src/gdb/target.c: In function ‘target_read_stralloc’:
../../src/gdb/target.c:2376:3: error: pointer targets in passing argument 1 of ‘strlen’ differ in signedness [-Werror=pointer-sign]
In file included from build-gnulib/import/string.h:27:0,
                 from ../../src/gdb/common/gdb_string.h:24,
                 from ../../src/gdb/target.c:24:
/usr/include/string.h:399:15: note: expected ‘const char *’ but argument is of type ‘gdb_byte *’
../../src/gdb/target.c: In function ‘target_fileio_read_stralloc’:
...

This is about the same as the previous patch.

Functions that take or return ascii-ish string arguments usually use
char* for parameters/return.  That means that at points we call into
target methods that work with binary blobs, we need casts to/from
gdb_byte*/char*.  To choose which type for the variables, I usually go
based on which requires the fewer casts, and what the contents of the
variable are supposed to hold, which often gives the same answer.

gdb/
2013-03-07  Pedro Alves  <palves@redhat.com>

	* target.c (target_read_stralloc, target_fileio_read_alloc):
	*Cast pointer to 'gdb_byte *' in target call.
---
 gdb/target.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index ecb1325..eaf8b31 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2359,10 +2359,11 @@ char *
 target_read_stralloc (struct target_ops *ops, enum target_object object,
 		      const char *annex)
 {
-  gdb_byte *buffer;
+  char *buffer;
   LONGEST i, transferred;
 
-  transferred = target_read_alloc_1 (ops, object, annex, &buffer, 1);
+  transferred = target_read_alloc_1 (ops, object, annex,
+				     (gdb_byte **) &buffer, 1);
 
   if (transferred < 0)
     return NULL;
@@ -2382,7 +2383,7 @@ target_read_stralloc (struct target_ops *ops, enum target_object object,
 	break;
       }
 
-  return (char *) buffer;
+  return buffer;
 }
 
 /* Memory transfer methods.  */
@@ -3522,10 +3523,11 @@ target_fileio_read_alloc (const char *filename, gdb_byte **buf_p)
 char *
 target_fileio_read_stralloc (const char *filename)
 {
-  gdb_byte *buffer;
+  char *buffer;
   LONGEST i, transferred;
 
-  transferred = target_fileio_read_alloc_1 (filename, &buffer, 1);
+  transferred = target_fileio_read_alloc_1 (filename,
+					    (gdb_byte **) &buffer, 1);
 
   if (transferred < 0)
     return NULL;
@@ -3545,7 +3547,7 @@ target_fileio_read_stralloc (const char *filename)
 	break;
       }
 
-  return (char *) buffer;
+  return buffer;
 }
 
 

Build regression on RHEL-5 [Re: [COMMIT PATCH] target.c: fix -Wpointer-sign]

[Jan Kratochvil]

On Fri, 08 Mar 2013 00:54:01 +0100, Pedro Alves wrote:
> gdb/
> 2013-03-07  Pedro Alves  <palves@redhat.com>
> 
> 	* target.c (target_read_stralloc, target_fileio_read_alloc):
> 	*Cast pointer to 'gdb_byte *' in target call.

53d2625aa5606dfd19ba3806eebeda54ce159f09

cc1: warnings being treated as errors
target.c: In function ‘target_read_stralloc’:
target.c:2370: warning: dereferencing type-punned pointer will break strict-aliasing rules
target.c: In function ‘target_fileio_read_stralloc’:
target.c:3534: warning: dereferencing type-punned pointer will break strict-aliasing rules

Tested on CentOS 5.9 x86_64 and i386.


Jan

Re: Build regression on RHEL-5 [Re: [COMMIT PATCH] target.c: fix -Wpointer-sign]

[Pedro Alves]

On 03/10/2013 06:55 PM, Jan Kratochvil wrote:
> On Fri, 08 Mar 2013 00:54:01 +0100, Pedro Alves wrote:
>> gdb/
>> 2013-03-07  Pedro Alves  <palves@redhat.com>
>>
>> 	* target.c (target_read_stralloc, target_fileio_read_alloc):
>> 	*Cast pointer to 'gdb_byte *' in target call.
> 
> 53d2625aa5606dfd19ba3806eebeda54ce159f09
> 
> cc1: warnings being treated as errors
> target.c: In function ‘target_read_stralloc’:
> target.c:2370: warning: dereferencing type-punned pointer will break strict-aliasing rules
> target.c: In function ‘target_fileio_read_stralloc’:
> target.c:3534: warning: dereferencing type-punned pointer will break strict-aliasing rules
> 
> Tested on CentOS 5.9 x86_64 and i386.

Thanks.  I'm aware of this (though Fedora 17's gcc doesn't warn at all), and I'll fix it Monday.

http://gcc.gnu.org/ml/gcc-help/2013-03/msg00118.html

The fix will be doing things like:

gdb_byte *buf;
char *c;
...
bufsiz = target_read_alloc (&current_target, TARGET_OBJECT_AVR,
			    "avr.io_reg", &buf);
c = (char *) buf;

-- 
Pedro Alves