Re: [gtk-vnc-devel] PATCH 1/2: Lifecycle re-factoring



Daniel P. Berrange wrote:
This first patch is just a little re-factoring to make the next patch
easier.
  - It makes all the gvnc related structs start with 'gvnc_' just for
    sake of consistency.

  - The gvnc_connect_* methods no longer actually allocate the gvnc
    data structure. They operate on a pre-existing object

  - A new gvnc_new method is avialable for allocating the struct

  - The gvnc_ops struct of callbacks is passed into the gvnc_new method
    straight away. This is because we need the callback right from the
    start for authentication callbacks

  - A gvnc_close() method can be used to free up data & reset internal
    state ready for opening a new connection

  - Adds in the vnc-disconnected  signal to vnc_display

In the future, it would be nice to split out some of these things into separate patches to make them easier to review. This changeset looks good though.

Regards,

Anthony Liguori

Dan
------------------------------------------------------------------------

changeset:   22:b77ea47300ab
user:        "Daniel P. Berrange <berrange redhat com>"
date:        Wed Jul 04 15:32:55 2007 -0400
summary:     Separate creation of gvnc object, from connect opening. Add ability to free/cleanup object & notify upon disconnect

diff -r 8a6239867d1e -r b77ea47300ab examples/gvncviewer.c
--- a/examples/gvncviewer.c	Wed Jul 04 14:55:43 2007 -0400
+++ b/examples/gvncviewer.c	Wed Jul 04 15:32:55 2007 -0400
@@ -34,6 +34,9 @@ int main(int argc, char **argv)
 	gtk_signal_connect(GTK_OBJECT(window), "delete-event",
 			   GTK_SIGNAL_FUNC(gtk_main_quit), NULL);
+ gtk_signal_connect(GTK_OBJECT(vnc), "vnc-disconnected",
+			   GTK_SIGNAL_FUNC(gtk_main_quit), NULL);
+
 	gtk_main();
return 0;
diff -r 8a6239867d1e -r b77ea47300ab examples/gvncviewer.py
--- a/examples/gvncviewer.py	Wed Jul 04 14:55:43 2007 -0400
+++ b/examples/gvncviewer.py	Wed Jul 04 15:32:55 2007 -0400
@@ -11,7 +11,8 @@ w.add(v)
 w.add(v)
 w.show_all()
-v.set_credential(gtkvnc.CREDENTIAL_PASSWORD, "123456")
-v.open_host("localhost", "5901", 0)
+v.set_password("123456")
+v.open_name("localhost", "5901")
+v.connect("vnc-disconnected", gtk.main_quit)
gtk.main()
diff -r 8a6239867d1e -r b77ea47300ab src/blt.h
--- a/src/blt.h	Wed Jul 04 14:55:43 2007 -0400
+++ b/src/blt.h	Wed Jul 04 15:32:55 2007 -0400
@@ -32,7 +32,7 @@ static void FILL(struct gvnc *gvnc, src_
 		 int x, int y, int width, int height)
 {
 	uint8_t *dst = gvnc_get_local(gvnc, x, y);
-	struct vnc_pixel_format *f = &gvnc->fmt;
+	struct gvnc_pixel_format *f = &gvnc->fmt;
 	int i;
for (i = 0; i < 1; i++) {
@@ -56,7 +56,7 @@ static void BLIT(struct gvnc *gvnc, uint
 static void BLIT(struct gvnc *gvnc, uint8_t *src, int pitch, int x, int y, int w, int h)
 {
 	uint8_t *dst = gvnc_get_local(gvnc, x, y);
-	struct vnc_pixel_format *f = &gvnc->fmt;
+	struct gvnc_pixel_format *f = &gvnc->fmt;
 	int i;
for (i = 0; i < h; i++) {
diff -r 8a6239867d1e -r b77ea47300ab src/gvnc.c
--- a/src/gvnc.c	Wed Jul 04 14:55:43 2007 -0400
+++ b/src/gvnc.c	Wed Jul 04 15:32:55 2007 -0400
@@ -70,7 +70,7 @@ struct gvnc
 	int fd;
 	char *host;
 	char *port;
-	struct vnc_pixel_format fmt;
+	struct gvnc_pixel_format fmt;
 	gboolean has_error;
 	int width;
 	int height;
@@ -88,7 +88,7 @@ struct gvnc
 	size_t write_offset;
gboolean perfect_match;
-	struct framebuffer local;
+	struct gvnc_framebuffer local;
int rp, gp, bp;
 	int rm, gm, bm;
@@ -98,7 +98,8 @@ struct gvnc
int shared_memory_enabled; - struct vnc_ops ops;
+	struct gvnc_ops ops;
+	gpointer ops_data;
int absolute;
 };
@@ -533,7 +534,7 @@ static int gvnc_validate_certificate(str
 }
-static void gvnc_read_pixel_format(struct gvnc *gvnc, struct vnc_pixel_format *fmt)
+static void gvnc_read_pixel_format(struct gvnc *gvnc, struct gvnc_pixel_format *fmt)
 {
 	uint8_t pad[3];
@@ -564,7 +565,7 @@ gboolean gvnc_has_error(struct gvnc *gvn
 }
gboolean gvnc_set_pixel_format(struct gvnc *gvnc,
-			       const struct vnc_pixel_format *fmt)
+			       const struct gvnc_pixel_format *fmt)
 {
 	uint8_t pad[3] = {0};
@@ -813,7 +814,7 @@ static void gvnc_update(struct gvnc *gvn
 {
 	if (gvnc->has_error || !gvnc->ops.update)
 		return;
-	gvnc->has_error = !gvnc->ops.update(gvnc->ops.user, x, y, width, height);
+	gvnc->has_error = !gvnc->ops.update(gvnc->ops_data, x, y, width, height);
 }
static void gvnc_set_color_map_entry(struct gvnc *gvnc, uint16_t color,
@@ -822,7 +823,7 @@ static void gvnc_set_color_map_entry(str
 {
 	if (gvnc->has_error || !gvnc->ops.set_color_map_entry)
 		return;
-	gvnc->has_error = !gvnc->ops.set_color_map_entry(gvnc->ops.user, color,
+	gvnc->has_error = !gvnc->ops.set_color_map_entry(gvnc->ops_data, color,
 							 red, green, blue);
 }
@@ -830,7 +831,7 @@ static void gvnc_bell(struct gvnc *gvnc)
 {
 	if (gvnc->has_error || !gvnc->ops.bell)
 		return;
-	gvnc->has_error = !gvnc->ops.bell(gvnc->ops.user);
+	gvnc->has_error = !gvnc->ops.bell(gvnc->ops_data);
 }
static void gvnc_server_cut_text(struct gvnc *gvnc, const void *data, @@ -838,28 +839,28 @@ static void gvnc_server_cut_text(struct {
 	if (gvnc->has_error || !gvnc->ops.server_cut_text)
 		return;
-	gvnc->has_error = !gvnc->ops.server_cut_text(gvnc->ops.user, data, len);
+	gvnc->has_error = !gvnc->ops.server_cut_text(gvnc->ops_data, data, len);
 }
static void gvnc_resize(struct gvnc *gvnc, int width, int height)
 {
 	if (gvnc->has_error || !gvnc->ops.resize)
 		return;
-	gvnc->has_error = !gvnc->ops.resize(gvnc->ops.user, width, height);
+	gvnc->has_error = !gvnc->ops.resize(gvnc->ops_data, width, height);
 }
static void gvnc_pointer_type_change(struct gvnc *gvnc, int absolute)
 {
 	if (gvnc->has_error || !gvnc->ops.pointer_type_change)
 		return;
-	gvnc->has_error = !gvnc->ops.pointer_type_change(gvnc->ops.user, absolute);
+	gvnc->has_error = !gvnc->ops.pointer_type_change(gvnc->ops_data, absolute);
 }
static void gvnc_shared_memory_rmid(struct gvnc *gvnc, int shmid)
 {
 	if (gvnc->has_error || !gvnc->ops.shared_memory_rmid)
 		return;
-	gvnc->has_error = !gvnc->ops.shared_memory_rmid(gvnc->ops.user, shmid);
+	gvnc->has_error = !gvnc->ops.shared_memory_rmid(gvnc->ops_data, shmid);
 }
static void gvnc_framebuffer_update(struct gvnc *gvnc, int32_t etype,
@@ -1302,6 +1303,66 @@ static gboolean gvnc_perform_auth(struct
 	return TRUE;
 }
+struct gvnc *gvnc_new(const struct gvnc_ops *ops, gpointer ops_data)
+{
+	struct gvnc *gvnc = malloc(sizeof(*gvnc));
+	if (gvnc == NULL)
+		return NULL;
+
+	memset(gvnc, 0, sizeof(*gvnc));
+	gvnc->fd = -1;
+
+	memcpy(&gvnc->ops, ops, sizeof(*ops));
+	gvnc->ops_data = ops_data;
+
+	return gvnc;
+}
+
+void gvnc_free(struct gvnc *gvnc)
+{
+	if (gvnc_is_connected(gvnc))
+		gvnc_close(gvnc);
+
+	free(gvnc);
+}
+
+void gvnc_close(struct gvnc *gvnc)
+{
+	if (gvnc->tls_session) {
+		gnutls_bye(gvnc->tls_session, GNUTLS_SHUT_RDWR);
+		gvnc->tls_session = NULL;
+	}
+	if (gvnc->channel) {
+		g_io_channel_unref(gvnc->channel);
+		gvnc->channel = NULL;
+	}
+	if (gvnc->fd != -1) {
+		close(gvnc->fd);
+		gvnc->fd = -1;
+	}
+
+	free(gvnc->host);
+	gvnc->host = NULL;
+
+	free(gvnc->port);
+	gvnc->port = NULL;
+
+	free(gvnc->name);
+	gvnc->name = NULL;
+
+	gvnc->has_error = 0;
+}
+
+gboolean gvnc_is_connected(struct gvnc *gvnc)
+{
+	if (gvnc->fd != -1)
+		return TRUE;
+	if (gvnc->host)
+		return TRUE;
+	return FALSE;
+}
+
+
 static gboolean gvnc_connect(struct gvnc *gvnc, gboolean shared_flag, const char *password)
 {
 	int ret;
@@ -1315,16 +1376,16 @@ static gboolean gvnc_connect(struct gvnc
ret = sscanf(version, "RFB %03d.%03d\n", &gvnc->major, &gvnc->minor);
 	if (ret != 2)
-		return FALSE;
+		goto fail;
if (gvnc->major != 3)
-		return FALSE;
+		goto fail;
 	if (gvnc->minor != 3 &&
 	    gvnc->minor != 5 &&
 	    gvnc->minor != 6 &&
 	    gvnc->minor != 7 &&
 	    gvnc->minor != 8)
-		return FALSE;
+		goto fail;
snprintf(version, 12, "RFB %03d.%03d\n", gvnc->major, gvnc->minor);
 	gvnc_write(gvnc, version, 12);
@@ -1333,7 +1394,7 @@ static gboolean gvnc_connect(struct gvnc
if (!gvnc_perform_auth(gvnc, password)) {
 		GVNC_DEBUG("Auth failed\n");
-		return FALSE;
+		goto fail;
 	}
gvnc_write_u8(gvnc, shared_flag); /* shared flag */
@@ -1345,63 +1406,50 @@ static gboolean gvnc_connect(struct gvnc
n_name = gvnc_read_u32(gvnc);
 	if (n_name > 4096)
-		return FALSE;
+		goto fail;
gvnc->name = malloc(n_name + 1);
 	if (gvnc->name == NULL)
-		return FALSE;
+		goto fail;
gvnc_read(gvnc, gvnc->name, n_name);
 	gvnc->name[n_name] = 0;
 	GVNC_DEBUG("Display name '%s'\n", gvnc->name);
gvnc_resize(gvnc, gvnc->width, gvnc->height);
-
-	return TRUE;
-}
-
-struct gvnc *gvnc_connect_fd(int fd,  gboolean shared_flag, const char *password)
-{
-	struct gvnc *gvnc = malloc(sizeof(*gvnc));
+	return gvnc_has_error(gvnc);
+
+ fail:
+	gvnc->has_error = 1;
+	return gvnc_has_error(gvnc);
+}
+
+gboolean gvnc_connect_fd(struct gvnc *gvnc, int fd,  gboolean shared_flag, const char *password)
+{
 	int flags;
-	if (gvnc == NULL)
-		return NULL;
-
-	memset(gvnc, 0, sizeof(*gvnc));
+	if (gvnc_is_connected(gvnc))
+		return TRUE;
GVNC_DEBUG("Connecting to FD %d\n", fd);
 	if ((flags = fcntl(fd, F_GETFL)) < 0)
-		goto error;
+		return TRUE;
 	flags |= O_NONBLOCK;
 	if (fcntl(fd, F_SETFL, flags) < 0)
-		goto error;
+		return TRUE;
if (!(gvnc->channel = g_io_channel_unix_new(fd)))
-		goto error;
+		return TRUE;
 	gvnc->fd = fd;
- if (!gvnc_connect(gvnc, shared_flag, password))
-		goto error;
-	return gvnc;
-
- error:
-	if (gvnc->fd)
-		close(gvnc->fd);
-	if (gvnc->channel)
-		g_io_channel_unref(gvnc->channel);
-	free(gvnc);
-	return NULL;
-}
-
-struct gvnc *gvnc_connect_name(const char *host, const char *port,  gboolean shared_flag, const char *password)
+	return gvnc_connect(gvnc, shared_flag, password);
+}
+
+gboolean gvnc_connect_name(struct gvnc *gvnc, const char *host, const char *port,  gboolean shared_flag, const char *password)
 {
         struct addrinfo *ai, *runp, hints;
         int ret;
-	struct gvnc *gvnc = malloc(sizeof(*gvnc));
-	if (gvnc == NULL)
-		return NULL;
-
-	memset(gvnc, 0, sizeof(*gvnc));
+	if (gvnc_is_connected(gvnc))
+		return TRUE;
GVNC_DEBUG("Resolving host %s %s\n", host, port);
         memset (&hints, '\0', sizeof (hints));
@@ -1410,7 +1458,7 @@ struct gvnc *gvnc_connect_name(const cha
         hints.ai_protocol = IPPROTO_TCP;
if ((ret = getaddrinfo(host, port, &hints, &ai)) != 0)
-		goto error;
+		return TRUE;
runp = ai;
         while (runp != NULL) {
@@ -1442,9 +1490,7 @@ struct gvnc *gvnc_connect_name(const cha
                         gvnc->channel = chan;
                         gvnc->fd = fd;
                         freeaddrinfo(ai);
-                        if (!gvnc_connect(gvnc, shared_flag, password))
-				goto error;
-			return gvnc;
+                        return gvnc_connect(gvnc, shared_flag, password);
                 }
if (errno == EINPROGRESS) {
@@ -1460,19 +1506,12 @@ struct gvnc *gvnc_connect_name(const cha
                 runp = runp->ai_next;
         }
         freeaddrinfo (ai);
-
- error:
-	if (gvnc->fd)
-		close(gvnc->fd);
-	if (gvnc->channel)
-		g_io_channel_unref(gvnc->channel);
-	free(gvnc);
-	return NULL;
-}
-
-
-
-gboolean gvnc_set_local(struct gvnc *gvnc, struct framebuffer *fb)
+	return TRUE;
+}
+
+
+
+gboolean gvnc_set_local(struct gvnc *gvnc, struct gvnc_framebuffer *fb)
 {
 	int i, j;
 	int depth;
@@ -1529,12 +1568,6 @@ gboolean gvnc_shared_memory_enabled(stru
 	return gvnc->shared_memory_enabled;
 }
-gboolean gvnc_set_vnc_ops(struct gvnc *gvnc, struct vnc_ops *ops)
-{
-	memcpy(&gvnc->ops, ops, sizeof(*ops));
-	gvnc_resize(gvnc, gvnc->width, gvnc->height);
-	return gvnc_has_error(gvnc);
-}
const char *gvnc_get_name(struct gvnc *gvnc)
 {
diff -r 8a6239867d1e -r b77ea47300ab src/gvnc.h
--- a/src/gvnc.h	Wed Jul 04 14:55:43 2007 -0400
+++ b/src/gvnc.h	Wed Jul 04 15:32:55 2007 -0400
@@ -6,7 +6,7 @@
struct gvnc; -struct vnc_ops
+struct gvnc_ops
 {
 	gboolean (*update)(void *, int, int, int, int);
 	gboolean (*set_color_map_entry)(void *, int, int, int, int);
@@ -15,10 +15,9 @@ struct vnc_ops
 	gboolean (*resize)(void *, int, int);
 	gboolean (*pointer_type_change)(void *, int);
 	gboolean (*shared_memory_rmid)(void *, int);
-	void *user;
 };
-struct vnc_pixel_format
+struct gvnc_pixel_format
 {
 	uint8_t bits_per_pixel;
 	uint8_t depth;
@@ -32,7 +31,7 @@ struct vnc_pixel_format
 	uint8_t blue_shift;
 };
-struct framebuffer
+struct gvnc_framebuffer
 {
 	uint8_t *data;
@@ -72,8 +71,13 @@ enum {
 	GVNC_ENCODING_SHARED_MEMORY = -258,
 };
-struct gvnc *gvnc_connect_fd(int fd, gboolean shared_flag, const char *password);
-struct gvnc *gvnc_connect_name(const char *host, const char *port, gboolean shared_flag, const char *password);
+struct gvnc *gvnc_new(const struct gvnc_ops *ops, gpointer ops_data);
+void gvnc_free(struct gvnc *gvnc);
+void gvnc_close(struct gvnc *gvnc);
+
+gboolean gvnc_is_connected(struct gvnc *gvnc);
+gboolean gvnc_connect_fd(struct gvnc *gvnc, int fd, gboolean shared_flag, const char *password);
+gboolean gvnc_connect_name(struct gvnc *gvnc, const char *host, const char *port, gboolean shared_flag, const char *password);
gboolean gvnc_server_message(struct gvnc *gvnc); @@ -93,15 +97,13 @@ gboolean gvnc_set_encodings(struct gvnc gboolean gvnc_set_encodings(struct gvnc *gvnc, int n_encoding, int32_t *encoding); gboolean gvnc_set_pixel_format(struct gvnc *gvnc,
-			       const struct vnc_pixel_format *fmt);
+			       const struct gvnc_pixel_format *fmt);
gboolean gvnc_set_shared_buffer(struct gvnc *gvnc, int line_size, int shmid); gboolean gvnc_has_error(struct gvnc *gvnc); -gboolean gvnc_set_local(struct gvnc *gvnc, struct framebuffer *fb);
-
-gboolean gvnc_set_vnc_ops(struct gvnc *gvnc, struct vnc_ops *ops);
+gboolean gvnc_set_local(struct gvnc *gvnc, struct gvnc_framebuffer *fb);
gboolean gvnc_shared_memory_enabled(struct gvnc *gvnc); diff -r 8a6239867d1e -r b77ea47300ab src/vncdisplay.c
--- a/src/vncdisplay.c	Wed Jul 04 14:55:43 2007 -0400
+++ b/src/vncdisplay.c	Wed Jul 04 15:32:55 2007 -0400
@@ -32,7 +32,7 @@ struct _VncDisplayPrivate
 	VncShmImage *shm_image;
 	GdkCursor *null_cursor;
- struct framebuffer fb;
+	struct gvnc_framebuffer fb;
 	struct coroutine coroutine;
 	struct gvnc *gvnc;
@@ -52,10 +52,11 @@ enum
 enum
 {
 	VNC_INITIALIZED,
+	VNC_DISCONNECTED,
 	LAST_SIGNAL
 };
-static guint signals[LAST_SIGNAL] = { 0 };
+static guint signals[LAST_SIGNAL] = { 0, 0 };
GtkWidget *vnc_display_new(void)
 {
@@ -369,6 +370,13 @@ static gboolean on_shared_memory_rmid(vo
 	return TRUE;
 }
+static const struct gvnc_ops vnc_display_ops = {
+	.update = on_update,
+	.resize = on_resize,
+	.pointer_type_change = on_pointer_type_change,
+	.shared_memory_rmid = on_shared_memory_rmid,
+};
+
 static void *vnc_coroutine(void *opaque)
 {
 	VncDisplay *obj = VNC_DISPLAY(opaque);
@@ -381,35 +389,41 @@ static void *vnc_coroutine(void *opaque)
 				GVNC_ENCODING_RAW };
int ret;
-	struct vnc_ops ops = {
-		.update = on_update,
-		.resize = on_resize,
-		.pointer_type_change = on_pointer_type_change,
-		.shared_memory_rmid = on_shared_memory_rmid,
-		.user = obj,
-	};
-
-	if (priv->fd != -1)
-		priv->gvnc = gvnc_connect_fd(priv->fd, FALSE, priv->password);
-	else
-		priv->gvnc = gvnc_connect_name(priv->host, priv->port, FALSE, priv->password);
+
+	priv->gvnc = gvnc_new(&vnc_display_ops, obj);
 	if (priv->gvnc == NULL)
 		return NULL;
+
+	if (priv->fd != -1) {
+		if (gvnc_connect_fd(priv->gvnc, priv->fd, FALSE, priv->password))
+			goto cleanup;
+	} else {
+		if (gvnc_connect_name(priv->gvnc, priv->host, priv->port, FALSE, priv->password))
+			goto cleanup;
+	}
g_signal_emit (G_OBJECT (obj),
 		       signals[VNC_INITIALIZED],
 		       0);
- gvnc_set_encodings(priv->gvnc, 6, encodings);
-
-	gvnc_set_vnc_ops(priv->gvnc, &ops);
-	gvnc_framebuffer_update_request(priv->gvnc, 0, 0, 0, priv->fb.width, priv->fb.height);
+	if (gvnc_set_encodings(priv->gvnc, 6, encodings))
+		goto cleanup;
+
+	if (gvnc_framebuffer_update_request(priv->gvnc, 0, 0, 0, priv->fb.width, priv->fb.height))
+		goto cleanup;
while (!(ret = gvnc_server_message(priv->gvnc))) {
-		gvnc_framebuffer_update_request(priv->gvnc, 1, 0, 0,
-						priv->fb.width, priv->fb.height);
-	}
-
+		if (gvnc_framebuffer_update_request(priv->gvnc, 1, 0, 0,
+						     priv->fb.width, priv->fb.height))
+			goto cleanup;
+	}
+
+ cleanup:
+	gvnc_free(priv->gvnc);
+	priv->gvnc = NULL;
+	g_signal_emit (G_OBJECT (obj),
+		       signals[VNC_DISCONNECTED],
+		       0);
 	return NULL;
 }
@@ -472,6 +486,16 @@ static void vnc_display_class_init(VncDi
 			      G_OBJECT_CLASS_TYPE (object_class),
 			      G_SIGNAL_RUN_FIRST,
 			      G_STRUCT_OFFSET (VncDisplayClass, vnc_initialized),
+			      NULL, NULL,
+			      g_cclosure_marshal_VOID__VOID,
+			      G_TYPE_NONE,
+			      0);
+
+	signals[VNC_DISCONNECTED] =
+		g_signal_new ("vnc-disconnected",
+			      G_OBJECT_CLASS_TYPE (object_class),
+			      G_SIGNAL_RUN_FIRST,
+			      G_STRUCT_OFFSET (VncDisplayClass, vnc_disconnected),
 			      NULL, NULL,
 			      g_cclosure_marshal_VOID__VOID,
 			      G_TYPE_NONE,
diff -r 8a6239867d1e -r b77ea47300ab src/vncdisplay.h
--- a/src/vncdisplay.h	Wed Jul 04 14:55:43 2007 -0400
+++ b/src/vncdisplay.h	Wed Jul 04 15:32:55 2007 -0400
@@ -48,6 +48,7 @@ struct _VncDisplayClass
/* Signals */
 	void		(* vnc_initialized)	(VncDisplay *display);
+	void		(* vnc_disconnected)	(VncDisplay *display);
int enter_grab_event_id;
 	int leave_grab_event_id;

------------------------------------------------------------------------

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
------------------------------------------------------------------------

_______________________________________________
Gtk-vnc-devel mailing list
Gtk-vnc-devel lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/gtk-vnc-devel





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