[PATCH weston 3/6] noop-renderer: Read the shm buffer contents on attach

[Emilio Pozuelo Monfort]

On 06/02/14 14:45, Pekka Paalanen wrote:
> On Thu,  6 Feb 2014 12:30:33 +0100
> Emilio Pozuelo Monfort <pochu27 at gmail.com> wrote:
> 
>> From: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>
>>
>> The noop-renderer doesn't read buffer contents, which means bad
>> buffers go undetected. Thus, read the buffer contents just for
>> the purpose of triggering SIGBUS (and having the client killed).
>>
>> Fixed bad-buffer test when run against the headless backend.
>>
>> Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>
>> ---
>>  src/noop-renderer.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/noop-renderer.c b/src/noop-renderer.c
>> index cf1a7f2..5da0b20 100644
>> --- a/src/noop-renderer.c
>> +++ b/src/noop-renderer.c
>> @@ -23,6 +23,7 @@
>>  #include "config.h"
>>  
>>  #include <stdlib.h>
>> +#include <string.h>
>>  
>>  #include "compositor.h"
>>  
>> @@ -50,15 +51,31 @@ static void
>>  noop_renderer_attach(struct weston_surface *es, struct weston_buffer *buffer)
>>  {
>>  	struct wl_shm_buffer *shm_buffer;
>> +	uint8_t *data;
>> +	uint32_t size, width, height, stride;
>>  
>>  	if (!buffer)
>>  		return;
>>  
>>  	shm_buffer = wl_shm_buffer_get(buffer->resource);
>>  
>> +	data = wl_shm_buffer_get_data(shm_buffer);
>> +	stride = wl_shm_buffer_get_stride(shm_buffer);
>> +	width = wl_shm_buffer_get_width(shm_buffer);
>> +	height = wl_shm_buffer_get_height(shm_buffer);
>> +	size = stride * height;
>> +
>> +	/* Access the buffer data to make sure the buffer's client gets killed
>> +	 * if the buffer size is invalid. This makes the bad_buffer test pass.
>> +	 * This can be removed if we start reading the buffer contents
>> +	 * somewhere else, e.g. in repaint_output(). */
>> +	wl_shm_buffer_begin_access(shm_buffer);
>> +	memset(data, 0, size);
>> +	wl_shm_buffer_end_access(shm_buffer);
>> +
>>  	buffer->shm_buffer = shm_buffer;
>> -	buffer->width = wl_shm_buffer_get_width(shm_buffer);
>> -	buffer->height = wl_shm_buffer_get_height(shm_buffer);
>> +	buffer->width = width;
>> +	buffer->height = height;
>>  }
>>  
>>  static void
> 
> Hi,
> 
> heh, nice, although *writing* to the client's buffer here is very
> naughty. ;-)
> 
> I'd prefer a reading loop here instead of memset.

I tried that first but it didn't work... because gcc was optimizing the code
away as the value was never used. I have solved that in v2 by using a volatile
variable as discussed on irc.

Cheers,
Emilio