[RFC wayland 01/18] shm: add getters for the fd and offset

[Pekka Paalanen]

On Tue,  9 Feb 2016 10:55:48 -0600
Derek Foreman <derekf at osg.samsung.com> wrote:

> From: Giulio Camuffo <giuliocamuffo at gmail.com>
> 
> This allows to share the buffer data by mmapping the fd again.
> Reviewed-by: David FORT <contact at hardening-consulting.com>
> 
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>  src/wayland-server-core.h |  6 ++++++
>  src/wayland-shm.c         | 16 +++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index e8e1e9c..3316022 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -480,6 +480,12 @@ wl_shm_buffer_create(struct wl_client *client,
>  		     uint32_t id, int32_t width, int32_t height,
>  		     int32_t stride, uint32_t format) WL_DEPRECATED;
>  
> +int
> +wl_shm_buffer_get_offset(struct wl_shm_buffer *buffer);
> +
> +int
> +wl_shm_buffer_get_fd(struct wl_shm_buffer *buffer);
> +
>  void wl_log_set_handler_server(wl_log_func_t handler);
>  
>  #ifdef  __cplusplus
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index a4343a4..911165d 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -55,6 +55,7 @@ struct wl_shm_pool {
>  	int refcount;
>  	char *data;
>  	int32_t size;
> +	int fd;
>  };
>  
>  struct wl_shm_buffer {
> @@ -80,6 +81,7 @@ shm_pool_unref(struct wl_shm_pool *pool)
>  		return;
>  
>  	munmap(pool->data, pool->size);
> +	close(pool->fd);
>  	free(pool);
>  }
>  
> @@ -253,7 +255,7 @@ shm_create_pool(struct wl_client *client, struct wl_resource *resource,
>  				       "failed mmap fd %d", fd);
>  		goto err_close;
>  	}
> -	close(fd);
> +	pool->fd = fd;
>  
>  	pool->resource =
>  		wl_resource_create(client, &wl_shm_pool_interface, 1, id);
> @@ -421,6 +423,18 @@ wl_shm_pool_unref(struct wl_shm_pool *pool)
>  	shm_pool_unref(pool);
>  }
>  
> +WL_EXPORT int
> +wl_shm_buffer_get_offset(struct wl_shm_buffer *buffer)
> +{
> +	return buffer->offset;
> +}
> +
> +WL_EXPORT int
> +wl_shm_buffer_get_fd(struct wl_shm_buffer *buffer)
> +{
> +	return buffer->pool->fd;
> +}
> +
>  static void
>  reraise_sigbus(void)
>  {

Hi,

Derek did wonder about this the last time with this patch, and I want
to reiterate, that leaving the pool fd open causes the server process
to have much much more open fds.

The server-side fds must be kept open also after the client has
destroyed the wl_shm_pool (and can no longer resize it), as the usual
usage pattern is to create a pool, create a wl_buffer, destroy the
pool. No fd is left open on the client, so the client is not in risk of
running out of open file slots.

Not only that, but a single server process is now keeping the shm fds
open from all clients.

We should carefully consider that before accepting this patch.

Maybe it's time to start lobbying for raising the open fd limits? We
use fds for so many things.


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: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160210/90a37f93/attachment.sig>