<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 17 July 2014 19:54, Jasper St. Pierre <span dir="ltr"><<a href="mailto:jstpierre@mecheye.net" target="_blank">jstpierre@mecheye.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The code here is wrong, leaky, and inconsistent. We don't free,<br>
unlink or clean up things when we should in every error path.<br>
<br>
Centralize the data destruction so it's easier to keep track of<br>
and easier to bug fix.<br>
---<br>
 src/wayland-server.c | 49 ++++++++++++++++++++++++-------------------------<br>
 1 file changed, 24 insertions(+), 25 deletions(-)<br>
<br>
diff --git a/src/wayland-server.c b/src/wayland-server.c<br>
index 1c9d4d0..3e0fb25 100644<br>
--- a/src/wayland-server.c<br>
+++ b/src/wayland-server.c<br>
@@ -840,6 +840,23 @@ wl_display_create(void)<br>
        return display;<br>
 }<br>
<br>
+static void<br>
+wl_socket_destroy(struct wl_socket *s)<br>
+{<br>
+       if (s->source)<br>
+               wl_event_source_remove(s->source);<br>
+       if (s->addr.sun_path[0])<br>
+               unlink(s->addr.sun_path);<br>
+       if (s->fd)<br>
+               close(s->fd);<br>
+       if (s->lock_addr[0])<br>
+               unlink(s->lock_addr);<br>
+       if (s->fd_lock)<br>
+               close(s->fd_lock);<br>
+<br>
+       free(s);<br>
+}<br>
+<br>
 WL_EXPORT void<br>
 wl_display_destroy(struct wl_display *display)<br>
 {<br>
@@ -849,12 +866,7 @@ wl_display_destroy(struct wl_display *display)<br>
        wl_signal_emit(&display->destroy_signal, display);<br>
<br>
        wl_list_for_each_safe(s, next, &display->socket_list, link) {<br>
-               wl_event_source_remove(s->source);<br>
-               unlink(s->addr.sun_path);<br>
-               close(s->fd);<br>
-               unlink(s->lock_addr);<br>
-               close(s->fd_lock);<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br>
        }<br>
        wl_event_loop_destroy(display->loop);<br>
<br>
@@ -1072,7 +1084,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
<br>
        s->fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);<br>
        if (s->fd < 0) {<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br></blockquote><div><br></div><div>Here you are using uninitialized values, since malloc is not zeroing memory and fd is the only filed set atm.<br></div><div>The same in the rest of wl_socket_destroy calls, until all fields are set. Memset or calloc should fix it.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
                return -1;<br>
        }<br>
<br>
@@ -1090,8 +1102,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
        if (name_size > (int)sizeof s->addr.sun_path) {<br>
                wl_log("error: socket path \"%s/%s\" plus null terminator"<br>
                       " exceeds 108 bytes\n", runtime_dir, name);<br>
-               close(s->fd);<br>
-               free(s);<br>
+               wl_socket_destroy(s); </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
                /* to prevent programs reporting<br>
                 * "failed to add socket: Success" */<br>
                errno = ENAMETOOLONG;<br>
@@ -1100,28 +1111,20 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
<br>
        s->fd_lock = get_socket_lock(s);<br>
        if (s->fd_lock < 0) {<br>
-               close(s->fd);<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br>
                return -1;<br>
        }<br>
<br>
        size = offsetof (struct sockaddr_un, sun_path) + name_size;<br>
        if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0) {<br>
                wl_log("bind() failed with error: %m\n");<br>
-               close(s->fd);<br>
-               unlink(s->lock_addr);<br>
-               close(s->fd_lock);<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br>
                return -1;<br>
        }<br>
<br>
        if (listen(s->fd, 1) < 0) {<br>
                wl_log("listen() failed with error: %m\n");<br>
-               unlink(s->addr.sun_path);<br>
-               close(s->fd);<br>
-               unlink(s->lock_addr);<br>
-               close(s->fd_lock);<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br>
                return -1;<br>
        }<br>
<br>
@@ -1129,11 +1132,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
                                         WL_EVENT_READABLE,<br>
                                         socket_data, display);<br>
        if (s->source == NULL) {<br>
-               unlink(s->addr.sun_path);<br>
-               close(s->fd);<br>
-               unlink(s->lock_addr);<br>
-               close(s->fd_lock);<br>
-               free(s);<br>
+               wl_socket_destroy(s);<br>
                return -1;<br>
        }<br>
        wl_list_insert(display->socket_list.prev, &s->link);<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.0.1<br>
<br>
_______________________________________________<br>
wayland-devel mailing list<br>
<a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br></font></span></blockquote><div><br></div><div>Otherwise looks OK.<br>
<br></div><div>Marek <br></div></div><br></div></div>