[PATCH wayland 1/3 v2] shm: Deprecate wl_shm_buffer_create()

[Pekka Paalanen]

On Sun, 4 Oct 2015 13:46:03 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> 2015-06-26 19:34 GMT+03:00 Derek Foreman <derekf at osg.samsung.com>:
> > From irc:
> > <pq> it creates a wl_buffer object in a way that no client can ever
> >      access the storage.
> >
> > So, let's replace it with abort(); and mark it with WL_DEPRECATED
> > in the header.
> >
> > Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> > ---
> >
> > This time using WL_DEPRECATED
> >
> >  src/wayland-server-core.h |  2 +-
> >  src/wayland-shm.c         | 29 +----------------------------
> >  2 files changed, 2 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > index e605432..dc87168 100644
> > --- a/src/wayland-server-core.h
> > +++ b/src/wayland-server-core.h
> > @@ -397,7 +397,7 @@ wl_display_add_shm_format(struct wl_display *display, uint32_t format);
> >  struct wl_shm_buffer *
> >  wl_shm_buffer_create(struct wl_client *client,
> >                      uint32_t id, int32_t width, int32_t height,
> > -                    int32_t stride, uint32_t format);
> > +                    int32_t stride, uint32_t format) WL_DEPRECATED;
> >
> >  void wl_log_set_handler_server(wl_log_func_t handler);
> >
> > diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> > index 5c419fa..a9a0b76 100644
> > --- a/src/wayland-shm.c
> > +++ b/src/wayland-shm.c
> > @@ -319,34 +319,7 @@ wl_shm_buffer_create(struct wl_client *client,
> >                      uint32_t id, int32_t width, int32_t height,
> >                      int32_t stride, uint32_t format)
> >  {
> > -       struct wl_shm_buffer *buffer;
> > -
> > -       if (!format_is_supported(client, format))
> > -               return NULL;
> > -
> > -       buffer = malloc(sizeof *buffer + stride * height);
> > -       if (buffer == NULL)
> > -               return NULL;
> > -
> > -       buffer->width = width;
> > -       buffer->height = height;
> > -       buffer->format = format;
> > -       buffer->stride = stride;
> > -       buffer->offset = 0;
> > -       buffer->pool = NULL;
> > -
> > -       buffer->resource =
> > -               wl_resource_create(client, &wl_buffer_interface, 1, id);
> > -       if (buffer->resource == NULL) {
> > -               free(buffer);
> > -               return NULL;
> > -       }
> > -
> > -       wl_resource_set_implementation(buffer->resource,
> > -                                      &shm_buffer_interface,
> > -                                      buffer, destroy_buffer);
> > -
> > -       return buffer;
> > +       abort();
> 
> 
> If we abort() here we make the function unusable and fatal and i'd
> argue that it may be better to just remove it instead, because no
> function is better than a pretend function that will kill you without
> arguing.

We can't remove it, because that is a major ABI break. Not when it can
be called without a crash at least. I suspect we'll have to keep the
stub forever.

> Maybe it's better to make it return NULL instead?

Yeah, that would be good enough.

NULL or abort(),
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Patch 2 needs a rewrite if you change anything here, but consider it
Acked-by me.

Patch 3 is Acked-by me too, I don't care too much if it's abort(),
assert(), return NULL or all of them. If we hit a NULL pool, there is
guaranteed a bug somewhere, so hopefully that would be caught ASAP
somehow.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151016/5f0c7cb2/attachment.sig>