fixed bug in wl_list

2011-03-11 Thread Iskren Chernev
Hello,

I found a bug and fixed it with the patch :)

*to reproduce:*
run compositor on top of x11

repeat
   run flower
   drag & drop it a little
   move the pointer in and out of the compositor/flower
   Ctrl+C the flower client

it would break eventually

*problem:*
I found that the linked list surface->destroy_listener_list got corrupted at
some point (it was not circular any more, strange next/prev etc), which
causes the crash.

*solution:*
The problem was in wl_list_remove -- when you erase an element, you don't
mark it as 'erased', by setting prev/next to NULL for example. Then if you
erase it again the list becomes corrupt. I nullified the prev/next and check
in the begining of wl_list_remove for not-in-list elements and just ignore
them. That seems to fix it.

Regards,
Iskren
From 15de592a8adb3007f813a278f00ddf4727f757f3 Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Sat, 12 Mar 2011 02:28:04 +0200
Subject: [PATCH] Fixed bug in wl_list.

---
 wayland/wayland-util.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/wayland/wayland-util.c b/wayland/wayland-util.c
index 3643274..fb60da2 100644
--- a/wayland/wayland-util.c
+++ b/wayland/wayland-util.c
@@ -44,8 +44,14 @@ wl_list_insert(struct wl_list *list, struct wl_list *elm)
 WL_EXPORT void
 wl_list_remove(struct wl_list *elm)
 {
+	if (elm->prev == NULL)
+		// element not in list
+		return;
+
 	elm->prev->next = elm->next;
 	elm->next->prev = elm->prev;
+
+	elm->prev = elm->next = NULL;
 }
 
 WL_EXPORT int
-- 
1.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: patches addressing error handling

2011-03-11 Thread Iskren Chernev
Ooops I didn't sent you the latest version of the final patch :)

On Fri, Mar 11, 2011 at 4:54 PM, Iskren Chernev wrote:

> Some resources were not freed if the create function exited earlier.
>
> I'm not sure how important these patches are. If they are I can make
> another pass on the code to fix similar errors. Also adding warn
> unused result to the creation functions that may fail will help
> spotting all places.
>
> Regards,
> Iskren
>
From a3a12fb522a8376a06c3fd7b81b923e02b1abc34 Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Fri, 11 Mar 2011 16:59:53 +0200
Subject: [PATCH] Server socket creation error handling.

---
 wayland/wayland-server.c |   41 +
 1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index faa1e1a..036958f 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -659,8 +659,10 @@ wl_display_add_socket(struct wl_display *display, const char *name)
 		return -1;
 
 	s->fd = socket(PF_LOCAL, SOCK_STREAM, 0);
-	if (s->fd < 0)
+	if (s->fd < 0) {
+		free(s);
 		return -1;
+	}
 
 	runtime_dir = getenv("XDG_RUNTIME_DIR");
 	if (runtime_dir == NULL) {
@@ -682,15 +684,27 @@ wl_display_add_socket(struct wl_display *display, const char *name)
 	fprintf(stderr, "using socket %s\n", s->addr.sun_path);
 
 	size = offsetof (struct sockaddr_un, sun_path) + name_size;
-	if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0)
+	if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0) {
+		close(s->fd);
+		free(s);
 		return -1;
+	}
 
-	if (listen(s->fd, 1) < 0)
+	if (listen(s->fd, 1) < 0) {
+		close(s->fd);
+		unlink(s->addr.sun_path);
+		free(s);
 		return -1;
+	}
 
-	wl_event_loop_add_fd(display->loop, s->fd,
-			 WL_EVENT_READABLE,
-			 socket_data, display);
+	if (wl_event_loop_add_fd(display->loop, s->fd,
+ WL_EVENT_READABLE,
+ socket_data, display) == NULL) {
+		close(s->fd);
+		unlink(s->addr.sun_path);
+		free(s);
+		return -1;
+	}
 	wl_list_insert(display->socket_list.prev, &s->link);
 
 	return 0;
@@ -710,23 +724,26 @@ wl_compositor_init(struct wl_compositor *compositor,
 	compositor->argb_visual.object.interface = &wl_visual_interface;
 	compositor->argb_visual.object.implementation = NULL;
 	wl_display_add_object(display, &compositor->argb_visual.object);
-	wl_display_add_global(display, &compositor->argb_visual.object, NULL);
+	if (wl_display_add_global(display, &compositor->argb_visual.object, NULL))
+		return -1;
 
 	compositor->premultiplied_argb_visual.object.interface =
 		&wl_visual_interface;
 	compositor->premultiplied_argb_visual.object.implementation = NULL;
 	wl_display_add_object(display,
 			  &compositor->premultiplied_argb_visual.object);
-	wl_display_add_global(display,
-			  &compositor->premultiplied_argb_visual.object,
-			  NULL);
+	if (wl_display_add_global(display,
+  &compositor->premultiplied_argb_visual.object,
+  NULL))
+		return -1;
 
 	compositor->rgb_visual.object.interface = &wl_visual_interface;
 	compositor->rgb_visual.object.implementation = NULL;
 	wl_display_add_object(display,
 			  &compositor->rgb_visual.object);
-	wl_display_add_global(display,
-			  &compositor->rgb_visual.object, NULL);
+	if (wl_display_add_global(display,
+  &compositor->rgb_visual.object, NULL))
+		return -1;
 
 	return 0;
 }
-- 
1.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


patches addressing error handling

2011-03-11 Thread Iskren Chernev
Some resources were not freed if the create function exited earlier.

I'm not sure how important these patches are. If they are I can make
another pass on the code to fix similar errors. Also adding warn
unused result to the creation functions that may fail will help
spotting all places.

Regards,
Iskren
From 57f2ef62b7c6c01a4aa1e6944812b97f0ec7b9b4 Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Fri, 11 Mar 2011 14:19:00 +0200
Subject: [PATCH 1/4] Better cleanup on display creation failure.

---
 wayland/wayland-server.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index dece0d1..17b91ae 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -528,6 +528,7 @@ wl_display_create(void)
 
 	display->objects = wl_hash_table_create();
 	if (display->objects == NULL) {
+		wl_event_loop_destroy(display->loop);
 		free(display);
 		return NULL;
 	}
@@ -544,6 +545,7 @@ wl_display_create(void)
 	wl_display_add_object(display, &display->object);
 	if (wl_display_add_global(display, &display->object, NULL)) {
 		wl_event_loop_destroy(display->loop);
+		wl_hash_table_destroy(display->objects);
 		free(display);
 		return NULL;
 	}
-- 
1.7.4

From 08972227552d0d777862064ea9b17277042bf4ef Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Fri, 11 Mar 2011 14:58:06 +0200
Subject: [PATCH 2/4] Better handling of creation errors in display.

---
 wayland/wayland-client.c |6 ++
 wayland/wayland-server.c |2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/wayland/wayland-client.c b/wayland/wayland-client.c
index ad35c30..0be1c47 100644
--- a/wayland/wayland-client.c
+++ b/wayland/wayland-client.c
@@ -373,6 +373,11 @@ wl_display_connect(const char *name)
 	}
 
 	display->objects = wl_hash_table_create();
+	if (display->objects == NULL) {
+		close(display->fd);
+		free(display);
+		return NULL;
+	}
 	wl_list_init(&display->global_listener_list);
 
 	display->proxy.object.interface = &wl_display_interface;
@@ -397,6 +402,7 @@ WL_EXPORT void
 wl_display_destroy(struct wl_display *display)
 {
 	wl_connection_destroy(display->connection);
+	wl_hash_table_destroy(display->objects);
 	close(display->fd);
 	free(display);
 }
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index 17b91ae..99b9f33 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -544,8 +544,8 @@ wl_display_create(void)
 	display->object.implementation = (void (**)(void)) &display_interface;
 	wl_display_add_object(display, &display->object);
 	if (wl_display_add_global(display, &display->object, NULL)) {
-		wl_event_loop_destroy(display->loop);
 		wl_hash_table_destroy(display->objects);
+		wl_event_loop_destroy(display->loop);
 		free(display);
 		return NULL;
 	}
-- 
1.7.4

From e535caf51fbf4823b082f1b382327d787f89b17e Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Fri, 11 Mar 2011 14:43:10 +0200
Subject: [PATCH 3/4] Added wl_connection_create failure checks.

---
 wayland/connection.c |2 ++
 wayland/wayland-client.c |7 ++-
 wayland/wayland-server.c |4 
 3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/wayland/connection.c b/wayland/connection.c
index 4a00470..0d705b5 100644
--- a/wayland/connection.c
+++ b/wayland/connection.c
@@ -160,6 +160,8 @@ wl_connection_create(int fd,
 	struct wl_connection *connection;
 
 	connection = malloc(sizeof *connection);
+	if (connection == NULL)
+		return NULL;
 	memset(connection, 0, sizeof *connection);
 	connection->fd = fd;
 	connection->update = update;
diff --git a/wayland/wayland-client.c b/wayland/wayland-client.c
index 0be1c47..1e9f915 100644
--- a/wayland/wayland-client.c
+++ b/wayland/wayland-client.c
@@ -394,7 +394,12 @@ wl_display_connect(const char *name)
 	display->connection = wl_connection_create(display->fd,
 		   connection_update,
 		   display);
-
+	if (display->connection == NULL) {
+		wl_hash_table_destroy(display->objects);
+		close(display->fd);
+		free(display);
+		return NULL;
+	}
 	return display;
 }
 
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index 99b9f33..faa1e1a 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -226,6 +226,10 @@ wl_client_create(struct wl_display *display, int fd)
 	  wl_client_connection_data, client);
 	client->connection =
 		wl_connection_create(fd, wl_client_connection_update, client);
+	if (client->connection == NULL) {
+		free(client);
+		return NULL;
+	}
 
 	wl_list_init(&client->resource_list);
 
-- 
1.7.4

From adc5e6b17e102b7e16869f082364572fb6a51cac Mon Sep 17 00:00:00 2001
From: Iskren Chernev 
Date: Fri, 11 Mar 2011 16:30:59 +0200
Subject: [PATCH 4/4] Server socket creation error handling.

---
 wayland/wayland-server.c |   26 --
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index faa1e1a..f554f9d 100644