[PATCH] Prevent zero sized wl_egl_window

[PATCH] Prevent zero sized wl_egl_window

[Sinclair Yeh]

It is illegal to create or resize a window to zero (or negative) width
and/or height.  This patch prevents such a request from happening.
---
 src/egl/wayland/wayland-egl/wayland-egl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c b/src/egl/wayland/wayland-egl/wayland-egl.c
index 8bd49cf..ae78595 100644
--- a/src/egl/wayland/wayland-egl/wayland-egl.c
+++ b/src/egl/wayland/wayland-egl/wayland-egl.c
@@ -9,6 +9,9 @@ wl_egl_window_resize(struct wl_egl_window *egl_window,
 		     int width, int height,
 		     int dx, int dy)
 {
+	if (width <= 0 || height <= 0)
+		return;
+
 	egl_window->width  = width;
 	egl_window->height = height;
 	egl_window->dx     = dx;
@@ -24,6 +27,9 @@ wl_egl_window_create(struct wl_surface *surface,
 {
 	struct wl_egl_window *egl_window;
 
+	if (width <= 0 || height <= 0)
+		return NULL;
+
 	egl_window = malloc(sizeof *egl_window);
 	if (!egl_window)
 		return NULL;
-- 
1.8.1.4

Re: [PATCH] Prevent zero sized wl_egl_window

[Pekka Paalanen]

On Wed, 12 Feb 2014 16:21:11 -0800
Sinclair Yeh <sinclair.yeh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> It is illegal to create or resize a window to zero (or negative) width
> and/or height.  This patch prevents such a request from happening.
> ---
>  src/egl/wayland/wayland-egl/wayland-egl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c
> b/src/egl/wayland/wayland-egl/wayland-egl.c index 8bd49cf..ae78595
> 100644 --- a/src/egl/wayland/wayland-egl/wayland-egl.c
> +++ b/src/egl/wayland/wayland-egl/wayland-egl.c
> @@ -9,6 +9,9 @@ wl_egl_window_resize(struct wl_egl_window *egl_window,
>  		     int width, int height,
>  		     int dx, int dy)
>  {
> +	if (width <= 0 || height <= 0)
> +		return;
> +

The below seems fine, but I wonder if we could make this one cause an
error to be returned later where we can, rather than silently ignoring.
I'm not sure where or how, though.

Surely drivers have maximum size limits, too, those must be catched
somewhere already.

>  	egl_window->width  = width;
>  	egl_window->height = height;
>  	egl_window->dx     = dx;
> @@ -24,6 +27,9 @@ wl_egl_window_create(struct wl_surface *surface,
>  {
>  	struct wl_egl_window *egl_window;
>  
> +	if (width <= 0 || height <= 0)
> +		return NULL;
> +
>  	egl_window = malloc(sizeof *egl_window);
>  	if (!egl_window)
>  		return NULL;

Thanks,
pq

Re: [PATCH] Prevent zero sized wl_egl_window

[Yeh, Sinclair]

> The below seems fine, but I wonder if we could make this one cause an
> error to be returned later where we can, rather than silently ignoring.
> I'm not sure where or how, though.

Would it make sense to change wl_egl_window_resize() so that it return a
value?  Either that, or it should be documented somewhere in the API
spec that setting width/height <=0 will be ignored.


> 
> Surely drivers have maximum size limits, too, those must be catched
> somewhere already.
> 
> >  	egl_window->width  = width;
> >  	egl_window->height = height;
> >  	egl_window->dx     = dx;
> > @@ -24,6 +27,9 @@ wl_egl_window_create(struct wl_surface *surface,
> >  {
> >  	struct wl_egl_window *egl_window;
> >  
> > +	if (width <= 0 || height <= 0)
> > +		return NULL;
> > +
> >  	egl_window = malloc(sizeof *egl_window);
> >  	if (!egl_window)
> >  		return NULL;
> 
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Prevent zero sized wl_egl_window

[Pekka Paalanen]

On Thu, 13 Feb 2014 18:18:23 +0000
"Yeh, Sinclair" <sinclair.yeh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:

> > The below seems fine, but I wonder if we could make this one cause an
> > error to be returned later where we can, rather than silently ignoring.
> > I'm not sure where or how, though.
> 
> Would it make sense to change wl_egl_window_resize() so that it return a
> value?  Either that, or it should be documented somewhere in the API
> spec that setting width/height <=0 will be ignored.

I'm not sure we can change the function signature, it's public stable
ABI.

> > Surely drivers have maximum size limits, too, those must be catched
> > somewhere already.

But this might be worth looking into: if the window system produces a
bad size, what do drivers do when they cannot allocate or render to it?

In X11 it's all hidden from the app, but I don't think the gfx stack can
guarantee valid sizes in all cases, can it?

Anyway, my suggestion is just for convenience, and if drivers already
just silently do whatever on a bad size, being silent here does not
make it any worse.


Thanks,
pq

> > 
> > >  	egl_window->width  = width;
> > >  	egl_window->height = height;
> > >  	egl_window->dx     = dx;
> > > @@ -24,6 +27,9 @@ wl_egl_window_create(struct wl_surface *surface,
> > >  {
> > >  	struct wl_egl_window *egl_window;
> > >  
> > > +	if (width <= 0 || height <= 0)
> > > +		return NULL;
> > > +
> > >  	egl_window = malloc(sizeof *egl_window);
> > >  	if (!egl_window)
> > >  		return NULL;
> > 
> > Thanks,
> > pq
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 

Re: [PATCH] Prevent zero sized wl_egl_window

[Kristian Høgsberg]

On Fri, Feb 14, 2014 at 09:31:45AM +0200, Pekka Paalanen wrote:
> On Thu, 13 Feb 2014 18:18:23 +0000
> "Yeh, Sinclair" <sinclair.yeh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> 
> > > The below seems fine, but I wonder if we could make this one cause an
> > > error to be returned later where we can, rather than silently ignoring.
> > > I'm not sure where or how, though.
> > 
> > Would it make sense to change wl_egl_window_resize() so that it return a
> > value?  Either that, or it should be documented somewhere in the API
> > spec that setting width/height <=0 will be ignored.
> 
> I'm not sure we can change the function signature, it's public stable
> ABI.
> 
> > > Surely drivers have maximum size limits, too, those must be catched
> > > somewhere already.
> 
> But this might be worth looking into: if the window system produces a
> bad size, what do drivers do when they cannot allocate or render to it?
> 
> In X11 it's all hidden from the app, but I don't think the gfx stack can
> guarantee valid sizes in all cases, can it?
> 
> Anyway, my suggestion is just for convenience, and if drivers already
> just silently do whatever on a bad size, being silent here does not
> make it any worse.

We could maybe make it raise an EGL error, I'm a little concerned with how
this silently fails.  However, the behavior with Sinclairs patch is 
better than what we had before, so I've pushed that.

> Thanks,
> pq
> 
> > > 
> > > >  	egl_window->width  = width;
> > > >  	egl_window->height = height;
> > > >  	egl_window->dx     = dx;
> > > > @@ -24,6 +27,9 @@ wl_egl_window_create(struct wl_surface *surface,
> > > >  {
> > > >  	struct wl_egl_window *egl_window;
> > > >  
> > > > +	if (width <= 0 || height <= 0)
> > > > +		return NULL;
> > > > +
> > > >  	egl_window = malloc(sizeof *egl_window);
> > > >  	if (!egl_window)
> > > >  		return NULL;
> > > 
> > > Thanks,
> > > pq
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel