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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 16.07.2017 um 02:31 schrieb Ramsay Jones:


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).

Can postimage->line be NULL when postimage->nr is bigger than 0?  What
would that mean?  The only ways to arrive at that point that I an come
up with are bugs (we accidentally set ->line to NULL, or we forgot to
clean ->line).  We'd better notice them early by getting a nice
shrieking segfault.  Adding an assert would work as well.

René



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux