[PATCH xinit] Simplify environment juggling by using fork() instead of vfork()

Alan Coopersmith alan.coopersmith at oracle.com
Tue Apr 13 08:31:41 PDT 2010


The code changes seem reasonable, and a nice simplification.

It does look like one area could be simplified even more:

> +        if (!setenv("WINDOWPATH", newwindowpath, TRUE)) {
> +            free(newwindowpath);
> +            return;
> +        }
> +
> +        free(newwindowpath);
>  }

Unless I'm missing something, both cases of the if result in the
same code - free + return.  I could see printing an error message
or something in the failure case, but then both should go to the
same free/return code.

Also, if we're no longer using HAVE_WORKING_VFORK & friends, then
you should also delete the AC_FUNC_FORK line from configure.ac

A couple formatting issues I note:

git am warned of a trailing whitespace issue when applying:
  .dotest/patch:106: trailing whitespace.
                snprintf(newwindowpath, len, "%s:%s",

It also seems like you've changed the indentation in startClient()
to not match the rest of the file (which appears to be 1 tab per
indent level mostly).   Though we have no consistent project-wide
style, we try to keep each source file consistent with itself at
least, to avoid really hurting our brains when reading them.

-- 
	-Alan Coopersmith-        alan.coopersmith at oracle.com
	 Oracle Solaris Platform Engineering: X Window System



More information about the xorg-devel mailing list