git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 2/6] Accept object data in the fsck_object() function

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 12, 2014, 08:07 UTC
Message-ID
<7bccb22f4c3e5aa45d3552e7acce1c360393a610.1410509168.git.johannes.schindelin@gmx.de>
In-Reply-To
<cover.1410509168.git.johannes.schindelin@gmx.de>

When fsck'ing an incoming pack, we need to fsck objects that cannot be read via read_sha1_file() because they are not local yet (and might even be rejected if transfer.fsckobjects is set to 'true').

For commits, there is a hack in place: we basically cache commit objects' buffers anyway, but the same is not true, say, for tag objects.

By refactoring fsck_object() to take the object buffer and size as optional arguments -- optional, because we still fall back to the previous method to look at the cached commit objects if the caller passes NULL -- we prepare the machinery for the upcoming handling of tag objects.

The assumption that such buffers are inherently NUL terminated is now wrong, of course, hence we pass the size of the buffer so that we can add a sanity check later, to prevent running past the end of the buffer.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/fsck.c           |  2 +-
 builtin/index-pack.c     |  3 ++-
 builtin/unpack-objects.c | 14 ++++++++++----
 fsck.c                   | 24 +++++++++++++++---------
 fsck.h                   |  4 +++-
 5 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index d42a27d..d9f4e6e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -298,7 +298,7 @@ static int fsck_obj(struct object *obj)
 
 	if (fsck_walk(obj, mark_used, NULL))
 		objerror(obj, "broken links");
-	if (fsck_object(obj, check_strict, fsck_error_func))
+	if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func))
 		return -1;
 
 	if (obj->type == OBJ_TREE) {
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 5568a5b..f2465ff 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -773,7 +773,8 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
 			if (!obj)
 				die(_("invalid %s"), typename(type));
 			if (do_fsck_object &&
-			    fsck_object(obj, 1, fsck_error_function))
+			    fsck_object(obj, buf, size, 1,
+				    fsck_error_function))
 				die(_("Error in object"));
 			if (fsck_walk(obj, mark_link, NULL))
 				die(_("Not all child objects of %s are reachable"), sha1_to_hex(obj->sha1));
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 99cde45..855d94b 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -164,10 +164,10 @@ static unsigned nr_objects;
  * Called only from check_object() after it verified this object
  * is Ok.
  */
-static void write_cached_object(struct object *obj)
+static void write_cached_object(struct object *obj, struct obj_buffer *obj_buf)
 {
 	unsigned char sha1[20];
-	struct obj_buffer *obj_buf = lookup_object_buffer(obj);
+
 	if (write_sha1_file(obj_buf->buffer, obj_buf->size, typename(obj->type), sha1) < 0)
 		die("failed to write object %s", sha1_to_hex(obj->sha1));
 	obj->flags |= FLAG_WRITTEN;
@@ -180,6 +180,8 @@ static void write_cached_object(struct object *obj)
  */
 static int check_object(struct object *obj, int type, void *data)
 {
+	struct obj_buffer *obj_buf;
+
 	if (!obj)
 		return 1;
 
@@ -198,11 +200,15 @@ static int check_object(struct object *obj, int type, void *data)
 		return 0;
 	}
 
-	if (fsck_object(obj, 1, fsck_error_function))
+	obj_buf = lookup_object_buffer(obj);
+	if (!obj_buf)
+		die("Whoops! Cannot find object '%s'", sha1_to_hex(obj->sha1));
+	if (fsck_object(obj, obj_buf->buffer, obj_buf->size, 1,
+			fsck_error_function))
 		die("Error in object");
 	if (fsck_walk(obj, check_object, NULL))
 		die("Error on reachable objects of %s", sha1_to_hex(obj->sha1));
-	write_cached_object(obj);
+	write_cached_object(obj, obj_buf);
 	return 0;
 }
 
diff --git a/fsck.c b/fsck.c
index 56156ff..dd77628 100644
--- a/fsck.c
+++ b/fsck.c
@@ -277,7 +277,7 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f
 }
 
 static int fsck_commit_buffer(struct commit *commit, const char *buffer,
-			      fsck_error error_func)
+	unsigned long size, fsck_error error_func)
 {
 	unsigned char tree_sha1[20], sha1[20];
 	struct commit_graft *graft;
@@ -322,15 +322,18 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,
 	return 0;
 }
 
-static int fsck_commit(struct commit *commit, fsck_error error_func)
+static int fsck_commit(struct commit *commit, const char *data,
+	unsigned long size, fsck_error error_func)
 {
-	const char *buffer = get_commit_buffer(commit, NULL);
-	int ret = fsck_commit_buffer(commit, buffer, error_func);
-	unuse_commit_buffer(commit, buffer);
+	const char *buffer = data ?  data : get_commit_buffer(commit, &size);
+	int ret = fsck_commit_buffer(commit, buffer, size, error_func);
+	if (!data)
+		unuse_commit_buffer(commit, buffer);
 	return ret;
 }
 
-static int fsck_tag(struct tag *tag, fsck_error error_func)
+static int fsck_tag(struct tag *tag, const char *data,
+	unsigned long size, fsck_error error_func)
 {
 	struct object *tagged = tag->tagged;
 
@@ -339,7 +342,8 @@ static int fsck_tag(struct tag *tag, fsck_error error_func)
 	return 0;
 }
 
-int fsck_object(struct object *obj, int strict, fsck_error error_func)
+int fsck_object(struct object *obj, void *data, unsigned long size,
+	int strict, fsck_error error_func)
 {
 	if (!obj)
 		return error_func(obj, FSCK_ERROR, "no valid object to fsck");
@@ -349,9 +353,11 @@ int fsck_object(struct object *obj, int strict, fsck_error error_func)
 	if (obj->type == OBJ_TREE)
 		return fsck_tree((struct tree *) obj, strict, error_func);
 	if (obj->type == OBJ_COMMIT)
-		return fsck_commit((struct commit *) obj, error_func);
+		return fsck_commit((struct commit *) obj, (const char *) data,
+			size, error_func);
 	if (obj->type == OBJ_TAG)
-		return fsck_tag((struct tag *) obj, error_func);
+		return fsck_tag((struct tag *) obj, (const char *) data,
+			size, error_func);
 
 	return error_func(obj, FSCK_ERROR, "unknown type '%d' (internal fsck error)",
 			  obj->type);
diff --git a/fsck.h b/fsck.h
index 1e4f527..d1e6387 100644
--- a/fsck.h
+++ b/fsck.h
@@ -28,6 +28,8 @@ int fsck_error_function(struct object *obj, int type, const char *fmt, ...);
  *    0		everything OK
  */
 int fsck_walk(struct object *obj, fsck_walk_func walk, void *data);
-int fsck_object(struct object *obj, int strict, fsck_error error_func);
+/* If NULL is passed for data, we assume the object is local and read it. */
+int fsck_object(struct object *obj, void *data, unsigned long size,
+	int strict, fsck_error error_func);
 
 #endif

-- 
2.0.0.rc3.9669.g840d1f9
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 45 of 68 in “Improve tag checking in fsck and with transfer.fsckobjects”
  1. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Aug 28, 2014
  2. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Aug 28, 2014
  3. Junio C HamanoAug 28, 2014
  4. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Aug 28, 2014
  5. Junio C HamanoAug 28, 2014
  6. Jeff KingAug 29, 2014
  7. Jeff KingAug 29, 2014
  8. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Aug 28, 2014
  9. Junio C HamanoAug 28, 2014
  10. Jeff KingAug 29, 2014
  11. 4/6 fsck: check tag objects' headersJohannes Schindelin, Aug 28, 2014
  12. Junio C HamanoAug 28, 2014
  13. Junio C HamanoAug 28, 2014
  14. Jeff KingAug 29, 2014
  15. Junio C HamanoAug 31, 2014
  16. Jeff KingSep 3, 2014
  17. Junio C HamanoSep 3, 2014
  18. Jeff KingSep 4, 2014
  19. Jeff KingAug 29, 2014
  20. Junio C HamanoSep 2, 2014
  21. Jeff KingSep 3, 2014
  22. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Aug 28, 2014
  23. 6/6 Make sure that index-pack --strict fails upon invalid tag objectsJohannes Schindelin, Aug 28, 2014
  24. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 10, 2014
  25. Johannes SchindelinSep 10, 2014
  26. Junio C HamanoSep 10, 2014
  27. Junio C HamanoSep 10, 2014
  28. Johannes SchindelinSep 11, 2014
  29. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 11, 2014
  30. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 11, 2014
  31. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 11, 2014
  32. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 11, 2014
  33. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 11, 2014
  34. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 11, 2014
  35. 6/6 Make sure that index-pack --strict checks tag objectsJohannes Schindelin, Sep 11, 2014
  36. Junio C HamanoSep 11, 2014
  37. Junio C HamanoSep 11, 2014
  38. 0/3 hash-object --literallyJunio C Hamano, Sep 11, 2014
  39. 1/3 hash-object: reduce file-scope staticsJunio C Hamano, Sep 11, 2014
  40. 2/3 hash-object: pass 'write_object' as a flagJunio C Hamano, Sep 11, 2014
  41. 3/3 hash-object: add --literally optionJunio C Hamano, Sep 11, 2014
  42. Johannes SchindelinSep 12, 2014
  43. 0/6 Improve tag checking in fsck and with transfer.fsckobjectsJohannes Schindelin, Sep 12, 2014
  44. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 12, 2014
  45. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 12, 2014
  46. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 12, 2014
  47. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 12, 2014
  48. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 12, 2014
  49. 6/6 Make sure that index-pack --strict checks tag objectsJohannes Schindelin, Sep 12, 2014
  50. Junio C HamanoSep 12, 2014
  51. Johannes SchindelinSep 13, 2014
  52. 1/6 Refactor type_from_string() to avoid die()ing in case of errorsJohannes Schindelin, Sep 10, 2014
  53. 2/6 Accept object data in the fsck_object() functionJohannes Schindelin, Sep 10, 2014
  54. 3/6 Make sure fsck_commit_buffer() does not run out of the bufferJohannes Schindelin, Sep 10, 2014
  55. Junio C HamanoSep 10, 2014
  56. Johannes SchindelinSep 11, 2014
  57. Junio C HamanoSep 11, 2014
  58. Eric SunshineSep 10, 2014
  59. 4/6 fsck: check tag objects' headersJohannes Schindelin, Sep 10, 2014
  60. Junio C HamanoSep 10, 2014
  61. 5/6 Add regression tests for stricter tag fsck'ingJohannes Schindelin, Sep 10, 2014
  62. Junio C HamanoSep 10, 2014
  63. Johannes SchindelinSep 11, 2014
  64. 6/6 Make sure that index-pack --strict fails upon invalid tag objectsJohannes Schindelin, Sep 10, 2014
  65. Junio C HamanoSep 10, 2014
  66. Johannes SchindelinSep 11, 2014
  67. Junio C HamanoSep 11, 2014
  68. Johannes SchindelinSep 11, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.