Web lists-archives.com

Re: [PATCH 3/2] apply: use COPY_ARRAY and MOVE_ARRAY in update_image()





On 15/07/17 21:20, René Scharfe wrote:
> Simplify the code by using the helper macros COPY_ARRAY and MOVE_ARRAY,
> which also makes them more robust in the case we copy or move no lines,
> as they allow using NULL points in that case, while memcpy(3) and
> memmove(3) don't.
> 
> Found with Clang's UBSan.
> 
> Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
> ---
> I don't know why the rules in contrib/coccinelle/array.cocci didn't
> match. :-?
> 
>  apply.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/apply.c b/apply.c
> index f2d599141d..40707ca50c 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -2809,13 +2809,10 @@ static void update_image(struct apply_state *state,
>  		img->line_allocated = img->line;
>  	}
>  	if (preimage_limit != postimage->nr)
> -		memmove(img->line + applied_pos + postimage->nr,
> -			img->line + applied_pos + preimage_limit,
> -			(img->nr - (applied_pos + preimage_limit)) *
> -			sizeof(*img->line));
> -	memcpy(img->line + applied_pos,
> -	       postimage->line,
> -	       postimage->nr * sizeof(*img->line));
> +		MOVE_ARRAY(img->line + applied_pos + postimage->nr,
> +			   img->line + applied_pos + preimage_limit,
> +			   img->nr - (applied_pos + preimage_limit));
> +	COPY_ARRAY(img->line + applied_pos, postimage->line, postimage->nr);

My patch looks like:

-       memcpy(img->line + applied_pos,
-              postimage->line,
-              postimage->nr * sizeof(*img->line));
+       if (postimage->line && postimage->nr)
+               memcpy(img->line + applied_pos,
+                      postimage->line,
+                      postimage->nr * sizeof(*img->line));

... which I think I prefer (slightly).


ATB,
Ramsay Jones

>  	if (!state->allow_overlap)
>  		for (i = 0; i < postimage->nr; i++)
>  			img->line[applied_pos + i].flag |= LINE_PATCHED;
>