Sorry to be later. Thanks for your comment. On Wed, 2014-04-09 at 09:59 +0200, Hardening wrote: > Le 08/04/2014 07:03, Quanxian Wang a écrit : > > Signed-off-by: Quanxian Wang <quanxian.w...@intel.com> > > --- > > module/Makefile.am | 3 + > > module/wrandr/Makefile.am | 32 ++ > > module/wrandr/wrandr.c | 1262 > > +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 1297 insertions(+) > > create mode 100644 module/Makefile.am > > create mode 100644 module/wrandr/Makefile.am > > create mode 100644 module/wrandr/wrandr.c > > > > diff --git a/module/Makefile.am b/module/Makefile.am new file mode > > 100644 index 0000000..1a10e65 > > --- /dev/null > > +++ b/module/Makefile.am > > @@ -0,0 +1,3 @@ > > +SUBDIRS = > > + > > +SUBDIRS += wrandr > > diff --git a/module/wrandr/Makefile.am b/module/wrandr/Makefile.am > > new file mode 100644 index 0000000..b0f2e6b > > --- /dev/null > > +++ b/module/wrandr/Makefile.am > > @@ -0,0 +1,32 @@ > > +moduledir = $(libdir)/weston > > +module_LTLIBRARIES = $(wrandr) > > + > > +AM_CPPFLAGS = \ > > + -I$(top_srcdir)/shared \ > > + -I$(top_srcdir)/src \ > > + -I$(top_builddir)/src \ > > + -DDATADIR='"$(datadir)"' \ > > + -DMODULEDIR='"$(moduledir)"' \ > > + -DLIBEXECDIR='"$(libexecdir)"' \ > > + -DIN_WESTON > > + > > [...] > > > +static void > > +update_region(struct weston_compositor *ec, > > + struct weston_output *target_output) { > > + pixman_region32_t old_output_region; > > + > > + /* Update output region and transformation matrix. */ > > + pixman_region32_init(&old_output_region); > > + pixman_region32_copy(&old_output_region, &target_output->region); > > + weston_output_transform_scale_init(target_output, > > + target_output->transform, > > + target_output->current_scale); > > + > > + pixman_region32_init(&target_output->previous_damage); > > + pixman_region32_init_rect(&target_output->region, > > + target_output->x, target_output->y, > > + target_output->width, target_output->height); > > + > > + weston_output_update_matrix(target_output); > > + > > + adjust_pointer(target_output, &old_output_region); > > + pixman_region32_fini(&old_output_region); > > + > > + /* Damage the output region in primary_plane */ > > + pixman_region32_union(&ec->primary_plane.damage, > > + &ec->primary_plane.damage, > > + &target_output->region); > > + > > + target_output->dirty = 1; > > +} > > + > > +static void > > +randr_switch_mode(struct randr_output_request *request, > > + uint32_t *results) > > +{ > > + time_t time = 0; > > + int i = 0, found = 0; > > + int result = WRANDR_TYPE_RET_NOACT; > > + int timing_mode = 0; > > + int move_bits = 0; > > + struct weston_mode *mode; > > + struct weston_output *output = request->output; > > + > > + if (request->flags & 1<<WRANDR_TYPE_OOP_MODENUM) { > > + timing_mode = 1; > > + time = request->modenum.modi_time; > > + *results |= result<< > > + (WRANDR_TYPE_OOP_MODENUM * 2); > > + } > > + > > + if (request->flags & 1<<WRANDR_TYPE_OOP_MODE) { > > + *results |= result<< > > + (WRANDR_TYPE_OOP_MODE * 2); > > + if (request->mode->modi_time > time) { > > + time = request->mode->modi_time; > > + timing_mode = 2; > > + } > > + } > > [Hardening] Perhaps the case where request->flags has not bits set > should be treated with an error returned ? No need to return error. In this function, we just check these two methods. Others will be processed in other function. If no flag, that is fine, no operation is needed. > > > + > > + switch (timing_mode) { > > + case 1: > > + i = 0; > > + wl_list_for_each(mode, &output->mode_list, link) { > > + i++; > > + if (i == request->modenum.num) { > > + found = 1; > > + break; > > + } > > + } > > + > > + if (found != 1) > > + mode = NULL; > > + break; > > + case 2: > > + mode = randr_find_mode(output, > > + request->mode->width, > > + request->mode->height, > > + request->mode->refresh); > > + break; > > + default: > > + return; > > + } > > [Hardening] related to my remark above, *results should be set in the > "default:" handling, as it catches the case where timing_mode == 0 > which means no flags where set. If timing_mode is 0, that means, no operation need to be done for switch mode. Just return is enough. > > > + > > + > > + if (!mode) { > > + result = WRANDR_TYPE_RET_FAIL; > > + } else { > > + if (!(mode->flags & WL_OUTPUT_MODE_CURRENT) && > > + output->switch_mode) { > > + result = output->switch_mode(output, mode); > > + if (result < 0) > > + result = WRANDR_TYPE_RET_FAIL; > > + else > > + result = WRANDR_TYPE_RET_SUCCESS; > > + } else > > + result = WRANDR_TYPE_RET_NOACT; > > + } > > + > > + move_bits = WRANDR_TYPE_OOP_MODENUM + timing_mode - 1; > > + *results &= ~(RESULT_MASK<<(move_bits * 2)); > > + *results |= result<<(move_bits * 2); } > > + > > > > [...] > > Regards >
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel