summaryrefslogtreecommitdiff
path: root/parallel-checkout.c
diff options
context:
space:
mode:
authorMatheus Tavares <matheus.bernardino@usp.br>2021-05-17 19:49:03 (GMT)
committerJunio C Hamano <gitster@pobox.com>2021-05-17 20:38:54 (GMT)
commit3d20ed27b8bdac5c82f4f78af802f9afa499f651 (patch)
tree4ac43f95e03a611d9e87630d9616d27cec0eefff /parallel-checkout.c
parentbf949ade81106fbda068c1fdb2c6fd1cb1babe7e (diff)
downloadgit-3d20ed27b8bdac5c82f4f78af802f9afa499f651.zip
git-3d20ed27b8bdac5c82f4f78af802f9afa499f651.tar.gz
git-3d20ed27b8bdac5c82f4f78af802f9afa499f651.tar.bz2
parallel-checkout: send the new object_id algo field to the workers
An object_id storing a SHA-1 name has some unused bytes at the end of the hash array. Since these bytes are not used, they are usually not initialized to any value either. However, at parallel_checkout.c:send_one_item() the object_id of a cache entry is copied into a buffer which is later sent to a checkout worker through a pipe write(). This makes Valgrind complain about passing uninitialized bytes to a syscall. The worker won't use these uninitialized bytes either, but the warning could confuse someone trying to debug this code; So instead of using oidcpy(), send_one_item() uses hashcpy() to only copy the used/initialized bytes of the object_id, and leave the remaining part with zeros. However, since cf0983213c ("hash: add an algo member to struct object_id", 2021-04-26), using hashcpy() is no longer sufficient here as it won't copy the new algo field from the object_id. Let's add and use a new function which meets both our requirements of copying all the important object_id data while still avoiding the uninitialized bytes, by padding the end of the hash array in the destination object_id. With this change, we also no longer need the destination buffer from send_one_item() to be initialized with zeros, so let's switch from xcalloc() to xmalloc() to make this clear. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'parallel-checkout.c')
-rw-r--r--parallel-checkout.c13
1 files changed, 6 insertions, 7 deletions
diff --git a/parallel-checkout.c b/parallel-checkout.c
index 6b1af32..ddc0ff3 100644
--- a/parallel-checkout.c
+++ b/parallel-checkout.c
@@ -411,7 +411,7 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
len_data = sizeof(struct pc_item_fixed_portion) + name_len +
working_tree_encoding_len;
- data = xcalloc(1, len_data);
+ data = xmalloc(len_data);
fixed_portion = (struct pc_item_fixed_portion *)data;
fixed_portion->id = pc_item->id;
@@ -421,13 +421,12 @@ static void send_one_item(int fd, struct parallel_checkout_item *pc_item)
fixed_portion->name_len = name_len;
fixed_portion->working_tree_encoding_len = working_tree_encoding_len;
/*
- * We use hashcpy() instead of oidcpy() because the hash[] positions
- * after `the_hash_algo->rawsz` might not be initialized. And Valgrind
- * would complain about passing uninitialized bytes to a syscall
- * (write(2)). There is no real harm in this case, but the warning could
- * hinder the detection of actual errors.
+ * We pad the unused bytes in the hash array because, otherwise,
+ * Valgrind would complain about passing uninitialized bytes to a
+ * write() syscall. The warning doesn't represent any real risk here,
+ * but it could hinder the detection of actual errors.
*/
- hashcpy(fixed_portion->oid.hash, pc_item->ce->oid.hash);
+ oidcpy_with_padding(&fixed_portion->oid, &pc_item->ce->oid);
variant = data + sizeof(*fixed_portion);
if (working_tree_encoding_len) {