On Fri, 5 Sep 2014 13:56:23 +0300
Pekka Paalanen <ppaala...@gmail.com> wrote:
> > + if (!backend) {
> > + if (getenv("WAYLAND_DISPLAY") ||
> > getenv("WAYLAND_SOCKET"))
> > + backend = strdup("wayland-backend.so");
> > + else if (getenv("DISPLAY"))
> > + backend = strdup("x11-backend.so");
> > + else
> > + backend = strdup(WESTON_NATIVE_BACKEND);
> > + }
>
> While working here, I think this piece of code could be extracted to a
> new function weston_choose_default_backend(). The extraction could be
> the first patch in the series.
>
> We might want to make the default backend choosing a bit smarter in the
> future, like not attempting to use backends we did not build.
>
> > }
> >
> > backend_init = weston_load_module(backend, "backend_init");
> > @@ -4440,24 +4437,26 @@ int main(int argc, char *argv[])
> > if (socket_name) {
> > if (wl_display_add_socket(display, socket_name)) {
> > weston_log("fatal: failed to add socket: %m\n");
> > - ret = EXIT_FAILURE;
> > - goto out;
> > + free(socket_name);
> > + socket_name = NULL;
> > }
> > } else {
> > socket_name = wl_display_add_socket_auto(display);
> > - if (!socket_name) {
> > + if (socket_name)
> > + socket_name = strdup(socket_name);
> > + else
> > weston_log("fatal: failed to add socket: %m\n");
> > - ret = EXIT_FAILURE;
> > - goto out;
> > - }
> > }
>
> I think we could extract another function here, that would include
> everything inside the else-clause of if (fd != -1), and call
> it e.g. int weston_create_listening_socket(display, socket_name).
> No need to return the final socket_name to main(), I believe.
>
> Then in main() you could just unconditionally call free(socket_name)
> once, since free(NULL) is ok.
>
> >
> > + if (!socket_name) {
> > + ret = EXIT_FAILURE;
> > + goto out;
> > + }
> > setenv("WAYLAND_DISPLAY", socket_name, 1);
> > + free(socket_name);
> > }
> >
> > - if (option_shell)
> > - shell = strdup(option_shell);
> > - else
> > + if (!shell)
> > weston_config_section_get_string(section, "shell", &shell,
> > "desktop-shell.so");
> >
> > @@ -4474,8 +4473,11 @@ int main(int argc, char *argv[])
> > }
> > free(modules);
> >
> > - if (load_modules(ec, option_modules, &argc, argv) < 0)
> > + if (load_modules(ec, option_modules, &argc, argv) < 0) {
> > + free(option_modules);
> > goto out;
> > + }
> > + free(option_modules);
> >
> > weston_compositor_wake(ec);
> >
>
> You could just have one free(option_modules) somewhere after out: label.
>
> We could collect all free()s for the WESTON_OPTION_STRING variables at
> the end of main().
>
>
> Thanks,
> pq
Hi, Pekka.
I sent a series of the version 2 of this patch, which fixes the issues you
mentioned.
I worried about the usage of not-free'd memory caused by collecting free()s at
the end of main().
But it will be very small and now I think it doesn't matter.
Thanks.
--
Ryo Munakata <ryomnk...@gmail.com>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
