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

["Daniel P. Berrange" <berrange redhat com>]

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

Dan
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

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;