On Fri, 16 Oct 2015 09:19:57 -0500 Derek Foreman <der...@osg.samsung.com> wrote:
> On 16/10/15 09:04 AM, Giulio Camuffo wrote: > > 2015-10-16 16:46 GMT+03:00 Pekka Paalanen <ppaala...@gmail.com>: > >> On Sun, 4 Oct 2015 13:46:03 +0300 > >> Giulio Camuffo <giuliocamu...@gmail.com> wrote: > >> > >>> 2015-06-26 19:34 GMT+03:00 Derek Foreman <der...@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 <der...@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. > > > > 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. > > Trying really hard to find a reason to disagree... It's theoretically > possible for a library to test for the presence of the function (dlopen, > assign a function pointer) but never actually use it. > > I think EFL does some clever tricks similar to this for some libraries. > > So a symbol that used to be present disappearing could be a break, even > if the function is never actually called. > > Maybe someone else has a more likely example of how removing this > utterly broken function would break a functional program. :) The dynamic linker ld.so with LD_BIND_NOW, nothing to do with dlopen() necessarily. Could be a piece of code calling this that itself is never called. I suspect someone (binary distributions?) might also be doing ABI checks based on exported symbols, and if a symbol disappears, it gets flagged as a break. I believe some projects (Mesa nowadays, right?) actually maintain lists of exported symbols to keep ABI. The one thing I am quite sure is that we must not remove this function. Whether it aborts or just always returns NULL, I don't care too much. Even if it's just to make ABI checking tools happy. Thanks, pq
pgp7jC4g8nPBC.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel