[PATCH v1] compositor-x11: fix title overflow in x11_backend_create_output

[Yong Bakos]

Hi Benoit,
Other than the comments inline below, this patch is
Reviewed-by: Yong Bakos <ybakos at humanoriented.com>
Tested-by: Yong Bakos <ybakos at humanoriented.com>

On Jun 4, 2016, at 9:08 AM, Benoit Gschwind <gschwind at gnu-log.net> wrote:
> 
> ---

I'd love to see a short message for the patch here explaining the
rationale and behavior change, now that it's possible to return NULL,
causing the compositor to terminate.


> src/compositor-x11.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 629b5f3..a518820 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -782,7 +782,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> {
> 	static const char name[] = "Weston Compositor";
> 	static const char class[] = "weston-1\0Weston Compositor";
> -	char title[32];
> +	char * title = NULL;

char *title = NULL;


> 	struct x11_output *output;
> 	xcb_screen_t *screen;
> 	struct wm_normal_hints normal_hints;
> @@ -800,11 +800,6 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> 	output_width = width * scale;
> 	output_height = height * scale;
> 
> -	if (configured_name)
> -		sprintf(title, "%s - %s", name, configured_name);
> -	else
> -		strcpy(title, name);
> -
> 	if (!no_input)
> 		values[0] |=
> 			XCB_EVENT_MASK_KEY_PRESS |
> @@ -871,9 +866,25 @@ x11_backend_create_output(struct x11_backend *b, int x, int y,
> 	}
> 
> 	/* Set window name.  Don't bother with non-EWMH WMs. */
> -	xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
> -			    b->atom.net_wm_name, b->atom.utf8_string, 8,
> -			    strlen(title), title);
> +	if (configured_name) {
> +		ret = asprintf(&title, "%s - %s", name, configured_name);
> +		 /* following asprintf manual, title is undefined on failure,
> +		  * thus force NULL */
> +		if (ret < 0)
> +			title = NULL;
> +	} else {
> +		title = strdup(name);
> +	}
> +
> +	if(title) {

if (title) {

> +		xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
> +				    b->atom.net_wm_name, b->atom.utf8_string, 8,
> +				    strlen(title), title);
> +		free(title);
> +	} else {
> +		return NULL;

Should we return NULL or just pass `name` to xcb_change_property?


> +	}
> +
> 	xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
> 			    b->atom.wm_class, b->atom.string, 8,
> 			    sizeof class, class);
> -- 
> 2.7.3

yong