[PATCH 2/2] Ensure strncpy results are null terminated.

Derek Foreman derekf at osg.samsung.com
Wed Sep 2 11:53:32 PDT 2015


On 28/08/15 03:04 PM, Bryce Harrington wrote:
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  clients/window.c       | 1 +
>  src/compositor-fbdev.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/clients/window.c b/clients/window.c
> index 0e73f5b..614b320 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -5886,6 +5886,7 @@ keysym_modifiers_add(struct wl_array *modifiers_map,
>  		return;
>  
>  	strncpy(p, name, len);
> +	p[len-1] = '\0';

In this case, len is defined to be strlen(name) + 1 further up in the
function, so strncpy will already be 0 padding to that - I think we were
already ok here?

However, maybe you want to make this just a strcpy() instead?  We
already know it's NULL terminated and we just allocated enough space for
it...

>  }
>  
>  static xkb_mod_index_t
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index a5c7947..b90975c 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -338,6 +338,7 @@ fbdev_query_screen_info(struct fbdev_output *output, int fd,
>  	info->buffer_length = fixinfo.smem_len;
>  	info->line_length = fixinfo.line_length;
>  	strncpy(info->id, fixinfo.id, sizeof(info->id) / sizeof(*info->id));
> +	info->id[sizeof(info->id)-1] = '\0';

This change looks good to me.

I took the liberty of submitting a patch to remove the odd /
sizeof(*info->id) bit (strncpy really wants bytes, not "array members" -
though here the array members are 1 byte in size...)

Your call if you want to respin yours on top of that or make me do a
follow up.  :)

Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
for this hunk.

>  
>  	info->pixel_format = calculate_pixman_format(&varinfo, &fixinfo);
>  	info->refresh_rate = calculate_refresh_rate(&varinfo);
> 



More information about the wayland-devel mailing list