2015-10-16 16:46 GMT+03:00 Pekka Paalanen <[email protected]>: > On Sun, 4 Oct 2015 13:46:03 +0300 > Giulio Camuffo <[email protected]> wrote: > >> 2015-06-26 19:34 GMT+03:00 Derek Foreman <[email protected]>: >> > 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 <[email protected]> >> > --- >> > >> > 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.
Imho making it abort() is an API break as well, in a sense, and a worse one, because it's at runtime instead of at compile time. The symbol is still there but any attempt at using it will punish you hard, so in a way it's like if it wasn't there, but you don't have the compiler complaining hard enough when you use it. -- Giulio > >> Maybe it's better to make it return NULL instead? > > Yeah, that would be good enough. > > NULL or abort(), > Reviewed-by: Pekka Paalanen <[email protected]> > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
