[PATCH wayland 3/6] server: Split out code to initialize the socket address for a display name

[Marek Chalupa]

It depends on implementation. For x86 bind is provided by syscall (
http://lxr.free-electrons.com/source/net/socket.c#L1521) and for example
for hurd on mach it's implemented in glibc (
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/mach/hurd/bind.c;h=a42b78ac07c7a72041f07650860567b6e9547a30;hb=2543fef229599e8a6e4feeea65ca2dd3f984154f#l33
).

The off-by-one probably won't break anything here, but I don't see any
reason why to use wrong value.

Cheers,
Marek


On 18 July 2014 14:29, Jasper St. Pierre <jstpierre at mecheye.net> wrote:

> Does the off-by-one matter? I don't think bind actually cares.
>
>
> On Fri, Jul 18, 2014 at 4:13 AM, Marek Chalupa <mchqwerty at gmail.com>
> wrote:
>
>>
>>
>>
>> On 17 July 2014 19:54, Jasper St. Pierre <jstpierre at mecheye.net> wrote:
>>
>>> We'll use this to autodetect a good socket to open on.
>>> ---
>>>  src/wayland-server.c | 41 ++++++++++++++++++++++++++---------------
>>>  1 file changed, 26 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/src/wayland-server.c b/src/wayland-server.c
>>> index d919eb2..3350751 100644
>>> --- a/src/wayland-server.c
>>> +++ b/src/wayland-server.c
>>> @@ -1060,11 +1060,9 @@ get_socket_lock(struct wl_socket *socket)
>>>         return fd_lock;
>>>  }
>>>
>>> -WL_EXPORT int
>>> -wl_display_add_socket(struct wl_display *display, const char *name)
>>> +static int
>>> +wl_socket_init_for_display_name(struct wl_socket *s, const char *name)
>>>  {
>>> -       struct wl_socket *s;
>>> -       socklen_t size;
>>>         int name_size;
>>>         const char *runtime_dir;
>>>
>>> @@ -1078,15 +1076,6 @@ wl_display_add_socket(struct wl_display *display,
>>> const char *name)
>>>                 return -1;
>>>         }
>>>
>>> -       s = malloc(sizeof *s);
>>> -       if (s == NULL)
>>> -               return -1;
>>> -
>>> -       if (name == NULL)
>>> -               name = getenv("WAYLAND_DISPLAY");
>>> -       if (name == NULL)
>>> -               name = "wayland-0";
>>> -
>>>         memset(&s->addr, 0, sizeof s->addr);
>>>         s->addr.sun_family = AF_LOCAL;
>>>         name_size = snprintf(s->addr.sun_path, sizeof s->addr.sun_path,
>>> @@ -1096,13 +1085,35 @@ wl_display_add_socket(struct wl_display
>>> *display, const char *name)
>>>         if (name_size > (int)sizeof s->addr.sun_path) {
>>>                 wl_log("error: socket path \"%s/%s\" plus null
>>> terminator"
>>>                        " exceeds 108 bytes\n", runtime_dir, name);
>>> -               wl_socket_destroy(s);
>>>                 /* to prevent programs reporting
>>>                  * "failed to add socket: Success" */
>>>                 errno = ENAMETOOLONG;
>>>                 return -1;
>>>         };
>>>
>>> +       return 0;
>>> +}
>>> +
>>> +WL_EXPORT int
>>> +wl_display_add_socket(struct wl_display *display, const char *name)
>>> +{
>>> +       struct wl_socket *s;
>>> +       socklen_t size;
>>> +
>>> +       s = malloc(sizeof *s);
>>> +       if (s == NULL)
>>> +               return -1;
>>> +
>>> +       if (name == NULL)
>>> +               name = getenv("WAYLAND_DISPLAY");
>>> +       if (name == NULL)
>>> +               name = "wayland-0";
>>> +
>>> +       if (wl_socket_init_for_display_name(s, name) < 0) {
>>> +               wl_socket_destroy(s);
>>> +               return -1;
>>> +       }
>>> +
>>>         s->fd_lock = get_socket_lock(s);
>>>         if (s->fd_lock < 0) {
>>>                 wl_socket_destroy(s);
>>> @@ -1115,7 +1126,7 @@ wl_display_add_socket(struct wl_display *display,
>>> const char *name)
>>>                 return -1;
>>>         }
>>>
>>> -       size = offsetof (struct sockaddr_un, sun_path) + name_size;
>>> +       size = offsetof (struct sockaddr_un, sun_path) +
>>> strlen(s->addr.sun_path);
>>>
>>
>> name_size in the original code is counting with the terminating zero,
>> strlen returns size of the string
>> excluding terminating zero, so here you are off by one.
>>
>> Maybe we could return name_size from the wl_socket_init_for_display_name,
>> it's returning int anyway
>> and we would reuse already counted value. However, I don't mind strlen.
>>
>>          if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0) {
>>>                 wl_log("bind() failed with error: %m\n");
>>>                 wl_socket_destroy(s);
>>> --
>>> 2.0.1
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
>> Regards,
>> Marek
>>
>
>
>
> --
>   Jasper
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140718/5094b40f/attachment.html>