| Message ID | 1392250871-3224-1-git-send-email-sinclair.yeh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 12 Feb 2014 16:21:11 -0800 Sinclair Yeh <sinclair.yeh@intel.com> 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
> 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
On Thu, 13 Feb 2014 18:18:23 +0000 "Yeh, Sinclair" <sinclair.yeh@intel.com> 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@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel >
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@intel.com> 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@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
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;