Re: [gtk-vnc-devel] PATCH: Fix bogus pointer arithmetic



Daniel P. Berrange wrote:
The gvnc_read function is doing arithmetic on a void* pointer. The size of
this is undefined by the C standard, though it happens to work with gcc
which assumes void* to be 1 byte doing arithmetic. Depending on the level of
warning flags enabled the compiler may complain about this anyway. So this
patch changes it to use  uint8_t *  throughout the I/O paths to get clearly
defined behaviour

I think gvnc_read/gvnc_write should take a void *. The proper fix IMHO is to change the char *ptr to uint8_t *ptr and just replace the instances of data + to ptr +.

That's the whole reason the ptr assignment is there, to avoid void * arithmetic.

Regards,

Anthony Liguori

Dan

diff -r fb8ab9818527 src/blt.h
--- a/src/blt.h	Fri Mar 07 11:24:19 2008 -0600
+++ b/src/blt.h	Fri Mar 07 12:36:10 2008 -0500
@@ -121,11 +121,11 @@ static void HEXTILE(struct gvnc *gvnc, u
 	} else {
 		/* Background Specified */
 		if (flags & 0x02)
-			gvnc_read(gvnc, bg, sizeof(*bg));
+			gvnc_read(gvnc, (uint8_t *)bg, sizeof(*bg));
/* Foreground Specified */
 		if (flags & 0x04)
-			gvnc_read(gvnc, fg, sizeof(*fg));
+			gvnc_read(gvnc, (uint8_t *)fg, sizeof(*fg));
if (gvnc->perfect_match)
 			FAST_FILL(gvnc, bg, x, y, width, height);
@@ -142,7 +142,7 @@ static void HEXTILE(struct gvnc *gvnc, u
/* SubrectsColored */
 				if (flags & 0x10)
-					gvnc_read(gvnc, fg, sizeof(*fg));
+					gvnc_read(gvnc, (uint8_t *)fg, sizeof(*fg));
xy = gvnc_read_u8(gvnc);
 				wh = gvnc_read_u8(gvnc);
diff -r fb8ab9818527 src/gvnc.c
--- a/src/gvnc.c	Fri Mar 07 11:24:19 2008 -0600
+++ b/src/gvnc.c	Fri Mar 07 12:36:10 2008 -0500
@@ -117,11 +117,11 @@ struct gvnc
 	char *cred_x509_cert;
 	char *cred_x509_key;
- char read_buffer[4096];
+	uint8_t read_buffer[4096];
 	size_t read_offset;
 	size_t read_size;
- char write_buffer[4096];
+	uint8_t write_buffer[4096];
 	size_t write_offset;
gboolean perfect_match;
@@ -148,7 +148,7 @@ struct gvnc
 	int wait_interruptable;
 	struct wait_queue wait;
- char *xmit_buffer;
+	uint8_t *xmit_buffer;
 	int xmit_buffer_capacity;
 	int xmit_buffer_size;
@@ -290,7 +290,7 @@ static gboolean gvnc_use_compression(str
 	return gvnc->compressed_buffer != NULL;
 }
-static int gvnc_zread(struct gvnc *gvnc, void *buffer, size_t size)
+static int gvnc_zread(struct gvnc *gvnc, uint8_t *buffer, size_t size)
 {
 	size_t offset = 0;
@@ -337,10 +337,10 @@ static int gvnc_zread(struct gvnc *gvnc, /* IO functions */ -static int gvnc_read(struct gvnc *gvnc, void *data, size_t len)
+static int gvnc_read(struct gvnc *gvnc, uint8_t *data, size_t len)
 {
 	int fd = g_io_channel_unix_get_fd(gvnc->channel);
-	char *ptr = data;
+	uint8_t *ptr = data;
 	size_t offset = 0;
if (gvnc->has_error) return -EINVAL;
@@ -456,9 +456,9 @@ static void gvnc_flush(struct gvnc *gvnc
 	gvnc->write_offset = 0;
 }
-static void gvnc_write(struct gvnc *gvnc, const void *data, size_t len)
-{
-	const char *ptr = data;
+static void gvnc_write(struct gvnc *gvnc, const uint8_t *data, size_t len)
+{
+	const uint8_t *ptr = data;
 	size_t offset = 0;
while (offset < len) {
@@ -544,21 +544,21 @@ static uint16_t gvnc_read_u16(struct gvn
 static uint16_t gvnc_read_u16(struct gvnc *gvnc)
 {
 	uint16_t value = 0;
-	gvnc_read(gvnc, &value, sizeof(value));
+	gvnc_read(gvnc, (uint8_t*)&value, sizeof(value));
 	return ntohs(value);
 }
static uint32_t gvnc_read_u32(struct gvnc *gvnc)
 {
 	uint32_t value = 0;
-	gvnc_read(gvnc, &value, sizeof(value));
+	gvnc_read(gvnc, (uint8_t *)&value, sizeof(value));
 	return ntohl(value);
 }
static int32_t gvnc_read_s32(struct gvnc *gvnc)
 {
 	int32_t value = 0;
-	gvnc_read(gvnc, &value, sizeof(value));
+	gvnc_read(gvnc, (uint8_t *)&value, sizeof(value));
 	return ntohl(value);
 }
@@ -570,19 +570,19 @@ static void gvnc_write_u16(struct gvnc *
 static void gvnc_write_u16(struct gvnc *gvnc, uint16_t value)
 {
 	value = htons(value);
-	gvnc_write(gvnc, &value, sizeof(value));
+	gvnc_write(gvnc, (uint8_t *)&value, sizeof(value));
 }
static void gvnc_write_u32(struct gvnc *gvnc, uint32_t value)
 {
 	value = htonl(value);
-	gvnc_write(gvnc, &value, sizeof(value));
+	gvnc_write(gvnc, (uint8_t *)&value, sizeof(value));
 }
static void gvnc_write_s32(struct gvnc *gvnc, int32_t value)
 {
 	value = htonl(value);
-	gvnc_write(gvnc, &value, sizeof(value));
+	gvnc_write(gvnc, (uint8_t *)&value, sizeof(value));
 }
#define DH_BITS 1024
@@ -2046,7 +2046,7 @@ gboolean gvnc_server_message(struct gvnc
 			break;
 		}
- gvnc_read(gvnc, data, n_text);
+		gvnc_read(gvnc, (uint8_t *)data, n_text);
 		data[n_text] = 0;
gvnc_server_cut_text(gvnc, data, n_text);
@@ -2077,7 +2077,7 @@ static gboolean gvnc_check_auth_result(s
 		len = gvnc_read_u32(gvnc);
 		if (len > (sizeof(reason)-1))
 			return FALSE;
-		gvnc_read(gvnc, reason, len);
+		gvnc_read(gvnc, (uint8_t *)reason, len);
 		reason[len] = '\0';
 		GVNC_DEBUG("Fail %s\n", reason);
 		if (!gvnc->has_error && gvnc->ops.auth_failure)
@@ -2712,7 +2712,7 @@ gboolean gvnc_initialize(struct gvnc *gv
gvnc->absolute = 1; - gvnc_read(gvnc, version, 12);
+	gvnc_read(gvnc, (uint8_t*)version, 12);
 	version[12] = 0;
ret = sscanf(version, "RFB %03d.%03d\n", &gvnc->major, &gvnc->minor);
@@ -2733,7 +2733,7 @@ gboolean gvnc_initialize(struct gvnc *gv
 	if  (gvnc->minor > 3 && gvnc->minor < 7) gvnc->minor = 3;
snprintf(version, 12, "RFB %03d.%03d\n", gvnc->major, gvnc->minor);
-	gvnc_write(gvnc, version, 12);
+	gvnc_write(gvnc, (uint8_t *)version, 12);
 	gvnc_flush(gvnc);
 	GVNC_DEBUG("Negotiated protocol %d %d\n", gvnc->major, gvnc->minor);
@@ -2757,7 +2757,7 @@ gboolean gvnc_initialize(struct gvnc *gv
 	if (gvnc->name == NULL)
 		goto fail;
- gvnc_read(gvnc, gvnc->name, n_name);
+	gvnc_read(gvnc, (uint8_t *)gvnc->name, n_name);
 	gvnc->name[n_name] = 0;
 	GVNC_DEBUG("Display name '%s'\n", gvnc->name);






[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]