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

From: Anthony Liguori <anthony codemonkey ws>
To: "Daniel P. Berrange" <berrange redhat com>
Cc: gtk-vnc-devel <gtk-vnc-devel lists sourceforge net>
Subject: Re: [gtk-vnc-devel] PATCH: Fix bogus pointer arithmetic
Date: Fri, 07 Mar 2008 11:54:46 -0600

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 IMHOis to change the char *ptr to uint8_t *ptr and just replace theinstances 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);

References:
- [gtk-vnc-devel] PATCH: Fix bogus pointer arithmetic
  - From: Daniel P. Berrange

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