[PATCH 03/10] compositor: Implement output configuration using windowed_output_api

[Pekka Paalanen]

On Fri, 12 Aug 2016 18:34:20 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Thu, 11 Aug 2016 17:33:58 +0200
> Armin Krezović <krezovic.armin at gmail.com> wrote:
> 
> > This implements output configuration for outputs which use
> > previously added weston_windowed_output_api. The function
> > takes an output that's to be configured, default configuration
> > that's to be set in case no configuration is specified in
> > the config file or on command line and optional third argument,
> > parsed_options, which will override defaults and options for
> > configuration if they are present.
> > 
> > This also introduces new compositor specific functions for
> > setting output's scale and transform from either hardcoded
> > default, config file option or command line option.
> > 
> > Pending output handling is also wired up in the compositor.  
> 
> Hi,
> 
> I wouldn't say "wired up" because wet_set_pending_output_handler() is
> not called yet in this patch. This patch is more like adding helpers
> for the windowed backend-cases.
> 
> There are a couple details pointed out below, but all in all this patch
> looks good to me. With the potential double-free fixed, you can slap a
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> on this patch.
> 
> > Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> > ---
> >  compositor/main.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 171 insertions(+)
> > 
> > diff --git a/compositor/main.c b/compositor/main.c
> > index 0e5af5b..c5d8e81 100644
> > --- a/compositor/main.c
> > +++ b/compositor/main.c

> > +static int
> > +wet_configure_windowed_output_from_config(struct weston_output *output,
> > +					  struct wet_output_config *defaults,
> > +					  struct wet_output_config *parsed_options)
> > +{
> > +	const struct weston_windowed_output_api *api =
> > +		weston_windowed_output_get_api(output->compositor);
> > +
> > +	struct weston_config *wc = wet_get_config(output->compositor);
> > +	struct weston_config_section *section = NULL;
> > +
> > +	int width = defaults->width;
> > +	int height = defaults->height;
> > +	int32_t scale = defaults->scale;
> > +	int32_t parsed_scale = 0;
> > +	uint32_t transform = defaults->transform;
> > +	uint32_t parsed_transform = UINT32_MAX;
> > +
> > +	if (!api) {
> > +		weston_log("Cannot use weston_windowed_output_api.\n");
> > +		return -1;
> > +	}
> > +
> > +	if (output->name)
> > +		section = weston_config_get_section(wc, "output", "name", output->name);
> > +
> > +	if (section) {
> > +		char *mode;
> > +
> > +		weston_config_section_get_string(section, "mode", &mode, NULL);
> > +		if (!mode || sscanf(mode, "%dx%d", &width,
> > +				    &height) != 2) {
> > +			weston_log("Invalid mode \"%s\" for output %s. Using defaults.\n",
> > +				   mode, output->name);
> > +			free(mode);  
> 
> Double-free.
> 
> 'mode' can be NULL.

Oh right, the "mode can be NULL" referred to the weston_log() call.
Passing NULL to a *printf as a %s is a bit sketchy, but at least glibc
deals with it alright, so I'll let it pass for now.

> 
> > +			width = defaults->width;
> > +			height = defaults->height;
> > +		}
> > +		free(mode);
> > +	}
> > +

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160816/c7b7390f/attachment.sig>