On Tue, 12 Apr 2016 21:34:28 -0700
Bryce Harrington <[email protected]> wrote:

> On Wed, Apr 06, 2016 at 11:37:57AM +0300, Pekka Paalanen wrote:
> > On Wed,  9 Mar 2016 16:49:29 -0800
> > Bryce Harrington <[email protected]> wrote:
> >   
> > > From: Giulio Camuffo <[email protected]>
> > > 
> > > Signed-off-by: Bryce Harrington <[email protected]>
> > > Reviewed-by: Quentin Glidic <[email protected]>
> > > Acked-by: Pekka Paalanen <[email protected]>
> > > ---
> > > v4: Update to current trunk
> > >     - Add missing param doc for mode in drm_output_choose_initial_mode
> > >     - Rebase to account for code changes by 91880f1e to make vt
> > >       switching configurable.
> > > 
> > >  Makefile.am          |   3 +
> > >  src/compositor-drm.c | 196 
> > > ++++++++++++++++++---------------------------------
> > >  src/compositor.h     |   2 -
> > >  src/main.c           |  94 +++++++++++++++++++++++-
> > >  4 files changed, 165 insertions(+), 130 deletions(-)  
> > 
> > Hi Giulio and Bryce,
> > 
> > I'm sorry it has taken so long for me to come back to this.

> > > +static int
> > > +load_drm_backend(struct weston_compositor *c, const char *backend,
> > > +          int *argc, char **argv, struct weston_config *wc)
> > > +{
> > > + struct drm_config *config;
> > > + struct weston_config_section *section;
> > > + int ret = 0;
> > > +
> > > + config = zalloc(sizeof *config);
> > > + if (!config)
> > > +         return -1;  
> > 
> > This function will be affected by the struct versioning too.  
> 
> The subsequent patch adds the versioning, although later in the routine
> than here.
> 
> > > + const struct weston_option options[] = {
> > > +         { WESTON_OPTION_INTEGER, "connector", 0, 
> > > &config->base.connector },
> > > +         { WESTON_OPTION_STRING, "seat", 0, &config->base.seat_id },
> > > +         { WESTON_OPTION_INTEGER, "tty", 0, &config->base.tty },
> > > +         { WESTON_OPTION_BOOLEAN, "current-mode", 0,
> > > +                                  &config->use_current_mode },
> > > +         { WESTON_OPTION_BOOLEAN, "use-pixman", 0, 
> > > &config->base.use_pixman },
> > > + };  
> > 
> > Mixed declarations and code.  
> 
> Hmm, I'm not sure best how to address this.  Options is being
> initialized with pointers that are created by the zalloc, so I don't
> think I can just separate the declaration from the initialization.  Does
> options need to be changed to be dynamically allocated as well?

Hi,

I suppose the simplest solution is to make another function, define
'options[]' in it, and get 'config' as an argument. Then it can call
the parser and return. Something like ...fill_from_command_line(struct
drm_config *config, argc, argv...).

It's fine to keep your changes as separate patches as long as every
patch is bisectable.

I'm going through the new series right now, and noticed these emails
from you just now.


Thanks,
pq

Attachment: pgpGK5wFG1unE.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to