On Fri, Apr 15, 2016 at 12:01:01PM +0300, Pekka Paalanen wrote: > On Thu, 14 Apr 2016 19:09:34 +0300 > Giulio Camuffo <[email protected]> wrote: > > > 2016-04-13 14:30 GMT+03:00 Pekka Paalanen <[email protected]>: > > > 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. > > > > I think this is one of the cases where strict style compliance goes in > > the way of better code. Adding a function just adds an indirect level > > but gives nothing in return, because it would really be just a wapper > > around the options[] variable. I don't see it much different to a "int > > value_1() { return 1; }". > > But more importantly imho, it requires another revision. > > No, it would be much more. The function would essentially be "fill in > this config struct from these argc,argv". That color would actually > please my eye more, as that is a logical single step to execute from a > load_*() function, rather than stuffing it all inline in load_*(). > > But, I'm not the one building the shed here. If I really care, I can > fix it after landing this stuff, since it doesn't seem to break the > build.
One wrinkle I'm spotting as I try to implement this is that the headless and x11 backends have options that are parsed but not stored in the config structure. Maybe I can shove them in anyway though. > Thanks, > pq > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > > iQIVAwUBVxCtzSNf5bQRqqqnAQiYHhAAmj+UCgMJGEj0vUglcKIx0FAsYQylqNm5 > zn0cYwGsi/mkbhCy5A6TNwTM5y21DGW7+/wcSwnlBZLH/7PDjX57ppwy+s7jcW5m > 9dLi/qpOlepfQ8S0BF1epFlQAyxBqa1JLvfI3O3ta91ku5VBCCEnKLNyqwCGE8z3 > COUudldrCP6Ysb9XY2TXLhlwDZxDu1uVeEXS2G1UxEj/ealB5RXypzuPKTSYVl77 > WsDhRgP1GMo4g1XL7B5cvBkvrrzDkj31u+M+pffIcvBJjnpL7ja7QSkerylXyaP7 > 1z4XIqM0vsyzxEnh7ZhNLIHcP1kOhunjJJi9/ti36DRwcL/flMIQoYElQU1MvBmw > Jv9tm2XlMjTIPMKgtdbSaz6fu6sJq0oEUQXYhpTMhaNx0H8Cg8uBpwQpaWsKP1A1 > QLM86aCpQsiNDPsvgTStX5M2hoJ0+MJbg/hpHy1ylhigaC3B8sx6nXX9JcGAEUBJ > DeNOruOjAsybOUPyBaKIufmiUt6pEJYRaCM0ghXMCYWqG1dqf0RlcGm1lK5z/F3F > 6SM+jciYqwN+/5yBWoUw0oeJiUbkoNnCgQwqfOKtitn2lPx3OY5seV9lBaLn4OnH > sF4HPAvIh7/2x/k19GN2q0sbueISUcoUmvPHqQg2Tj4JalU0ByWrRBlMtHoaIPvL > qM3v+5S/lP4= > =z6pB > -----END PGP SIGNATURE----- _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
