[PATCH] shadow_copy2: revert unnecessary talloc_zero_array initialization

Michael Adam obnox at samba.org
Wed Dec 11 01:54:01 MST 2013


Fix 2 bugs:
- 1. fix Volker's mail address
- 2. attach the patch ;-)



On 2013-12-11 at 09:52 +0100, Michael Adam wrote:
> Hi,
> 
> after a discussion with Volker, I added a unnecessary
> talloc_zero_array initialization when working on the
> shadow copy module. Since this expensive, I revert it
> and  add a comment describing this pitfall (as a separate
> commit to give the option not to pick this comment :-).
> 
> comments / review / push appreciated!
> 
> Cheers - Michael
> 


-------------- next part --------------
From 601cf6b45444e988699467f2cdd9375b42a2414c Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 11 Dec 2013 09:34:47 +0100
Subject: [PATCH 1/2] shadow_copy2: revert expensive and unnecessary
 zero-initialization

I was being overly cautious. This is initialization is not
necessary, since further down in the for-loop, the memory
always gets fully initialized because the insert string is
inserted at various slash positions.

So this talloc_zero_array can be skipped: this an expensive
thing to do in virtually every VFS call.

This essentially reverts commit 249e9b4a34d8959bd94735c1921ecfc24d6a2705.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/modules/vfs_shadow_copy2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 8243f63..7f5aeb2 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -482,7 +482,7 @@ static char *shadow_copy2_convert(TALLOC_CTX *mem_ctx,
 	}
 	insertlen = talloc_get_size(insert)-1;
 
-	converted = talloc_zero_array(mem_ctx, char, pathlen + insertlen + 1);
+	converted = talloc_array(mem_ctx, char, pathlen + insertlen + 1);
 	if (converted == NULL) {
 		goto fail;
 	}
-- 
1.7.9.5


From 6bcf86c00b70b635c34efc78c7564e74f0f29f38 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 11 Dec 2013 09:41:38 +0100
Subject: [PATCH 2/2] shadow_copy2: add a comment explaining why we don't
 talloc_zero_array().

Since I stumbled over this slighly sublte point, I thought it is
worthwile to point it our in a comment.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/modules/vfs_shadow_copy2.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 7f5aeb2..fca05cf 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -482,6 +482,15 @@ static char *shadow_copy2_convert(TALLOC_CTX *mem_ctx,
 	}
 	insertlen = talloc_get_size(insert)-1;
 
+	/*
+	 * Note: We deliberatly don't expensively initialize the
+	 * array with talloc_zero here: Putting zero into
+	 * converted[pathlen+insertlen] below is sufficient, because
+	 * in the following for loop, the insert string is inserted
+	 * at various slash places. So the memory up to position
+	 * pathlen+insertlen will always be initialized when the
+	 * converted string is used.
+	 */
 	converted = talloc_array(mem_ctx, char, pathlen + insertlen + 1);
 	if (converted == NULL) {
 		goto fail;
-- 
1.7.9.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131211/ae761998/attachment.pgp>


More information about the samba-technical mailing list