<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 7, 2014 at 9:24 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Comments below,<br>--Jason Ekstrand<br><br><div class="gmail_extra"><div class="gmail_quote"><div class="">
On Wed, May 7, 2014 at 9:25 AM, 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">We'll use this to autodetect a good socket to open on.<br>
---<br>
src/wayland-server.c | 40 +++++++++++++++++++++++++---------------<br>
1 file changed, 25 insertions(+), 15 deletions(-)<br>
<br>
diff --git a/src/wayland-server.c b/src/wayland-server.c<br>
index d0fd280..6bc8dc3 100644<br>
--- a/src/wayland-server.c<br>
+++ b/src/wayland-server.c<br>
@@ -1039,11 +1039,9 @@ get_socket_lock(struct wl_socket *socket)<br>
return fd_lock;<br>
}<br>
<br>
-WL_EXPORT int<br>
-wl_display_add_socket(struct wl_display *display, const char *name)<br>
+static int<br>
+open_socket_for_display_name(struct wl_socket *s, const char *name)<br></blockquote></div><div><br>Perhaps, this should be named differently. How about
wl_socket_try_lock. The problem is that it doesn't really open the
socket (assuming I put this and the previous patch together correctly in
my head)</div></div></div></div></blockquote><div><br></div><div>This function fills in the sun_path given the DISPLAY_NAME and tries the lock. Perhaps these should be separate: split out the sun_path stuff to setup_socket_for_display_name. Maybe we could combine them as try_socket_lock_for_display_name.<br>
</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
{<br>
- struct wl_socket *s;<br>
- socklen_t size;<br>
int name_size;<br>
const char *runtime_dir;<br>
<br>
@@ -1057,15 +1055,6 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
return -1;<br>
}<br>
<br>
- s = malloc(sizeof *s);<br>
- if (s == NULL)<br>
- return -1;<br>
-<br>
- if (name == NULL)<br>
- name = getenv("WAYLAND_DISPLAY");<br>
- if (name == NULL)<br>
- name = "wayland-0";<br>
-<br>
memset(&s->addr, 0, sizeof s->addr);<br>
s->addr.sun_family = AF_LOCAL;<br>
name_size = snprintf(s->addr.sun_path, sizeof s->addr.sun_path,<br>
@@ -1075,7 +1064,6 @@ 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>
- free(s);<br>
/* to prevent programs reporting<br>
* "failed to add socket: Success" */<br>
errno = ENAMETOOLONG;<br></blockquote></div></div><div><br>Why are we filling addr structure here and doing nothing with it? Perhaps filling the addr structure should stay in wl_display_add_socket?<br></div>
</div></div></div></blockquote><div><br></div><div>We are doing something with it. get_socket_lock uses it to find the correct lock name to take.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
@@ -1084,6 +1072,28 @@ 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>
+ return -1;<br>
+ }<br></blockquote><div><br></div></div><div>This is right before opening the socket, so it returns with an invalid value in s->fd.<br></div></div></div></div></blockquote><div><br></div><div>Hm? s->fd is opened below taking the lock in wl_display_add_socket. s->fd is untouched from its initial value, which should always be 0.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
+<br>
+ return 0;<br>
+}<br>
+<br>
+WL_EXPORT int<br>
+wl_display_add_socket(struct wl_display *display, const char *name)<br>
+{<br>
+ struct wl_socket *s;<br>
+ socklen_t size;<br>
+<br>
+ s = malloc(sizeof *s);<br>
+ if (s == NULL)<br>
+ return -1;<br>
+<br>
+ if (name == NULL)<br>
+ name = getenv("WAYLAND_DISPLAY");<br>
+ if (name == NULL)<br>
+ name = "wayland-0";<br>
+<br>
+ if (open_socket_for_display_name(s, name) < 0) {<br>
free(s);<br>
return -1;<br>
}<br>
@@ -1095,7 +1105,7 @@ wl_display_add_socket(struct wl_display *display, const char *name)<br>
return -1;<br>
}<br>
<br>
- size = offsetof (struct sockaddr_un, sun_path) + name_size;<br>
+ size = offsetof (struct sockaddr_un, sun_path) + strlen(s->addr.sun_path);<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>
</div></div><span class="HOEnZb"><font color="#888888"><span><font color="#888888">--<br>
1.9.0<br>
<br>
_______________________________________________<br>
wayland-devel mailing list<br>
<a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">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></font></span></blockquote></div><br></div></div>
</blockquote></div><br><br clear="all"><br>-- <br> Jasper<br>
</div></div>