On Thu, 2015-03-12 at 00:40 +0900, Ryo Munakata wrote: > On Wed, 11 Mar 2015 16:05:55 +0200 > Pekka Paalanen <ppaala...@gmail.com> wrote: > > > On Tue, 10 Mar 2015 11:34:45 +0900 > > Ryo Munakata <ryomnk...@gmail.com> wrote: > > > > > This was a TODO: > > > "Ideally, available frame buffers should be enumerated > > > using udev, rather than passing a device node in as a > > > parameter." > > > > Hi, > > > > I'm CC'ing the person responsible for the TODO-comment. > > > > > > > > Signed-off-by: Ryo Munakata <ryomnk...@gmail.com> > > > --- > > > src/compositor-fbdev.c | 55 > > > +++++++++++++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 52 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c > > > index eedf252..c0e6985 100644 > > > --- a/src/compositor-fbdev.c > > > +++ b/src/compositor-fbdev.c > > > @@ -46,6 +46,8 @@ > > > #include "gl-renderer.h" > > > #include "presentation_timing-server-protocol.h" > > > > > > +#define FBDEV_MAJOR_NUMBER (29) > > > + > > > struct fbdev_compositor { > > > struct weston_compositor base; > > > uint32_t prev_state; > > > @@ -690,6 +692,9 @@ fbdev_output_destroy(struct weston_output *base) > > > gl_renderer->output_destroy(base); > > > } > > > > > > + if (output->device) > > > + free((char *)output->device); > > > > No need to cast. Also no need for 'if' because free(NULL) is perfectly > > legal. > > This cast causes a warning like: > src/compositor-fbdev.c:696:7: warning: passing argument 1 of ‘free’ discards > ‘const’ qualifier from pointer target type > I think we need it for silencing the warning. > What do you think? > > And yes, that if statement is unnecessary. I'll fix it.
Rather than casting, you should remove the const qualifier from struct fbdev_output.device. > > > + > > > /* Remove the output. */ > > > weston_output_destroy(&output->base); > > > > > > @@ -748,6 +753,7 @@ fbdev_output_reenable(struct fbdev_compositor > > > *compositor, > > > * the frame buffer X/Y resolution (such as the shadow buffer) > > > * are re-initialised. */ > > > device = output->device; > > > + output->device = NULL; > > > > Why NULLing it here? Seem like this would make repeated reenable to > > fail, no? > > Here we reuse output->device for the new output. (device = output->device;) > output->device is allocated by strdup() dynamically and > fbdev_output_destroy() frees it if not NULL. > So if you don't set output->device as NULL we need to reallocate and copy the > entire device string for the new output everytime this function is called. > This is why I set output->device as NULL to prevent it. That seems like a hacky way of doing things. You would be better off refactoring the code out of fbdev_output_create() which initalises the struct fbdev_output (after it’s allocated), and just calling that here. Philip > > > fbdev_output_destroy(base); > > > fbdev_output_create(compositor, device); > > > > > Thank you.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel