[PATCH v2] os: Add -displayfd option

Alan Coopersmith alan.coopersmith at oracle.com
Tue Apr 3 20:13:38 PDT 2012


On 04/ 3/12 02:25 PM, Chase Douglas wrote:
> +static Bool
> +TryCreateSocket(int num, int *partial)
> +{
> +    char port[20];
> +
> +    sprintf(port, "%d", num);
> +
> +    return (_XSERVTransMakeAllCOTSServerListeners(port, partial,
> +                                                  &ListenTransCount,
> +                                                  &ListenTransConns) >= 0);
> +}
> +

I'd prefer if this stayed with snprintf like the old code did:

> -    snprintf(port, sizeof(port), "%d", atoi(display));

Not because there's any chance of it overflowing, but because it's one less
place we have to check to be sure sprintf can't overflow.

> +    if (display) {
[...]
> +    }
> +    else { /* -displayfd */

I had to go digging through the code before I could be convinced this was
a safe assumption to make.   Perhaps a comment somewhere like dix/globals.c
should state that display is initialized to "0" by main, and then set by
ProcessCommandLine to either a command line specified display, or in the
case of -displayfd to NULL, would be helpful for future understanding.

Definitely in favor of adding this support though, thanks for finishing it off.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc


More information about the xorg-devel mailing list