[PATCH 0/6] object-file: move logic related to the loose backend

[PATCH 0/6] object-file: move logic related to the loose backend

[Patrick Steinhardt]

Hi,

this small patch series moves logic that relates to the loose backend
from "object-file.c" into "odb/source-loose.c". On the one hand this
makes the backend more self-contained. On the other hand, this also lets
us quickly identify exactly those code units that still access internal
implementation details of the "loose" backend by grepping for those
files that include "odb/source-loose.h".

The second reason is in fact the main driver. I've got two more patch
series pending that do the same for the "packed" and "files" backends.
With those changes in it becomes trivial to do a quick "reality" check
of which subsystems are fully backend agnostic and which ones still need
more work.

The series is built on top of ebdc087dc99 (The 13th batch, 2026-11-01),
but it conflicts with bc/restrict-hex-to-lowercase. This conflict can be
easily resolved by deleting the conflicting section in "object-file.c" and
adding this diff in "odb/source-loose.c":

diff --git a/odb/source-loose.c b/odb/source-loose.c
index 54f680d6d78..e92da0d8b2e 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -81,7 +81,7 @@ static int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 		strbuf_add(path, de->d_name, namelen);
 		if (namelen == algop->hexsz - 2 &&
 		    !hex_to_bytes(oid.hash + 1, de->d_name,
-				  algop->rawsz - 1)) {
+				  algop->rawsz - 1, HEX_KIND_OID)) {
 			oid_set_algo(&oid, algop);
 			memset(oid.hash + algop->rawsz, 0,
 			       GIT_MAX_RAWSZ - algop->rawsz);

Thanks!

Patrick

---
Patrick Steinhardt (6):
      object-file: move logic to read loose objects into backend
      object-file: move logic to derive loose object paths into backend
      object-file: move iteration over loose files into backend
      odb/source-loose: deduplicate logic to mmap objects
      object-file: stop including "odb/source-loose.h"
      object-file: stop including "git-zlib.h"

 apply.c                 |   1 +
 builtin/count-objects.c |   1 +
 builtin/fast-import.c   |   1 +
 builtin/gc.c            |   1 +
 builtin/index-pack.c    |   1 +
 builtin/pack-objects.c  |   2 +
 builtin/prune.c         |   1 +
 diff.c                  |   1 +
 http-walker.c           |   1 +
 http.c                  |   1 +
 loose.c                 |   1 +
 object-file.c           | 419 +---------------------------------------------
 object-file.h           | 100 +----------
 odb/source-loose.c      | 429 ++++++++++++++++++++++++++++++++++++++++++++++--
 odb/source-loose.h      |  41 +++++
 pack-check.c            |   1 +
 packfile.c              |   1 +
 prune-packed.c          |   1 +
 setup.c                 |   1 +
 19 files changed, 476 insertions(+), 529 deletions(-)


---
base-commit: ebdc087dc99d68bb340c282b2ba5f722def28d19
change-id: 20261013-pks-odb-loose-cleanups-fb7e5b890389

[PATCH 1/6] object-file: move logic to read loose objects into backend

[Patrick Steinhardt]

The logic to read loose objects is still contained in "object-file.c"
even though it's an implementation detail of the "loose" backend, and
that backend is nowadays also the only remaining caller.

Move the logic around and remove the accompanying declarations.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c      | 300 ---------------------------------------------------
 object-file.h      |  45 --------
 odb/source-loose.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 306 insertions(+), 345 deletions(-)

diff --git a/object-file.c b/object-file.c
index 132aaab6f84..c1b78a25c9f 100644
--- a/object-file.c
+++ b/object-file.c
@@ -151,179 +151,6 @@ int stream_object_signature(struct repository *r,
 	return !oideq(oid, &real_oid) ? -1 : 0;
 }
 
-/*
- * Map and close the given loose object fd. The path argument is used for
- * error reporting.
- */
-static void *map_fd(int fd, const char *path, unsigned long *size)
-{
-	void *map = NULL;
-	struct stat st;
-
-	if (!fstat(fd, &st)) {
-		*size = xsize_t(st.st_size);
-		if (!*size) {
-			/* mmap() is forbidden on empty files */
-			error(_("object file %s is empty"), path);
-			close(fd);
-			return NULL;
-		}
-		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
-	}
-	close(fd);
-	return map;
-}
-
-enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
-						    unsigned char *map,
-						    unsigned long mapsize,
-						    void *buffer,
-						    unsigned long bufsiz)
-{
-	int status;
-
-	/* Get the data stream */
-	memset(stream, 0, sizeof(*stream));
-	stream->next_in = map;
-	stream->avail_in = mapsize;
-	stream->next_out = buffer;
-	stream->avail_out = bufsiz;
-
-	git_inflate_init(stream);
-	obj_read_unlock();
-	status = git_inflate(stream, 0);
-	obj_read_lock();
-	if (status != Z_OK && status != Z_STREAM_END)
-		return ULHR_BAD;
-
-	/*
-	 * Check if entire header is unpacked in the first iteration.
-	 */
-	if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
-		return ULHR_OK;
-
-	/*
-	 * We have a header longer than MAX_HEADER_LEN.
-	 */
-	return ULHR_TOO_LONG;
-}
-
-void *unpack_loose_rest(git_zstream *stream,
-			void *buffer, unsigned long size,
-			const struct object_id *oid)
-{
-	size_t bytes = strlen(buffer) + 1, n;
-	unsigned char *buf = xmallocz(size);
-	int status = Z_OK;
-
-	n = stream->total_out - bytes;
-	if (n > size)
-		n = size;
-	memcpy(buf, (char *) buffer + bytes, n);
-	bytes = n;
-	if (bytes <= size) {
-		/*
-		 * The above condition must be (bytes <= size), not
-		 * (bytes < size).  In other words, even though we
-		 * expect no more output and set avail_out to zero,
-		 * the input zlib stream may have bytes that express
-		 * "this concludes the stream", and we *do* want to
-		 * eat that input.
-		 *
-		 * Otherwise we would not be able to test that we
-		 * consumed all the input to reach the expected size;
-		 * we also want to check that zlib tells us that all
-		 * went well with status == Z_STREAM_END at the end.
-		 */
-		stream->next_out = buf + bytes;
-		stream->avail_out = size - bytes;
-		while (status == Z_OK) {
-			obj_read_unlock();
-			status = git_inflate(stream, Z_FINISH);
-			obj_read_lock();
-		}
-	}
-
-	if (status != Z_STREAM_END) {
-		error(_("corrupt loose object '%s'"), oid_to_hex(oid));
-		FREE_AND_NULL(buf);
-	} else if (stream->avail_in) {
-		error(_("garbage at end of loose object '%s'"),
-		      oid_to_hex(oid));
-		FREE_AND_NULL(buf);
-	}
-
-	return buf;
-}
-
-/*
- * parse_loose_header() parses the starting "<type> <len>\0" of an
- * object. If it doesn't follow that format -1 is returned. To check
- * the validity of the <type> populate the "typep" in the "struct
- * object_info". It will be OBJ_BAD if the object type is unknown. The
- * parsed <len> can be retrieved via "oi->sizep", and from there
- * passed to unpack_loose_rest().
- *
- * We used to just use "sscanf()", but that's actually way
- * too permissive for what we want to check. So do an anal
- * object header parse by hand.
- */
-int parse_loose_header(const char *hdr, struct object_info *oi)
-{
-	const char *type_buf = hdr;
-	size_t size;
-	int type, type_len = 0;
-
-	/*
-	 * The type can be of any size but is followed by
-	 * a space.
-	 */
-	for (;;) {
-		char c = *hdr++;
-		if (!c)
-			return -1;
-		if (c == ' ')
-			break;
-		type_len++;
-	}
-
-	type = type_from_string_gently(type_buf, type_len, 1);
-	if (oi->typep)
-		*oi->typep = type;
-
-	/*
-	 * The length must follow immediately, and be in canonical
-	 * decimal format (ie "010" is not valid).
-	 */
-	size = *hdr++ - '0';
-	if (size > 9)
-		return -1;
-	if (size) {
-		for (;;) {
-			unsigned long c = *hdr - '0';
-			if (c > 9)
-				break;
-			hdr++;
-			size = st_add(st_mult(size, 10), c);
-		}
-	}
-
-	if (oi->sizep)
-		*oi->sizep = size;
-
-	/*
-	 * The length must be followed by a zero byte
-	 */
-	if (*hdr)
-		return -1;
-
-	/*
-	 * The format is valid, but the type may still be bogus. The
-	 * Caller needs to check its oi->typep.
-	 */
-	return 0;
-}
-
 #define CHECK_COLLISION_DEST_VANISHED -2
 
 static int check_collision(const char *source, const char *dest)
@@ -1127,133 +954,6 @@ int for_each_loose_file_in_source(struct odb_source *source,
 	return r;
 }
 
-static int check_stream_oid(git_zstream *stream,
-			    const char *hdr,
-			    unsigned long size,
-			    const char *path,
-			    const struct object_id *expected_oid,
-			    const struct git_hash_algo *algop)
-{
-	struct git_hash_ctx c;
-	struct object_id real_oid;
-	unsigned char buf[4096];
-	unsigned long total_read;
-	int status = Z_OK;
-
-	git_hash_init(&c, algop);
-	git_hash_update(&c, hdr, stream->total_out);
-
-	/*
-	 * We already read some bytes into hdr, but the ones up to the NUL
-	 * do not count against the object's content size.
-	 */
-	total_read = stream->total_out - strlen(hdr) - 1;
-
-	/*
-	 * This size comparison must be "<=" to read the final zlib packets;
-	 * see the comment in unpack_loose_rest for details.
-	 */
-	while (total_read <= size &&
-	       (status == Z_OK ||
-		(status == Z_BUF_ERROR && !stream->avail_out))) {
-		stream->next_out = buf;
-		stream->avail_out = sizeof(buf);
-		if (size - total_read < stream->avail_out)
-			stream->avail_out = size - total_read;
-		status = git_inflate(stream, Z_FINISH);
-		git_hash_update(&c, buf, stream->next_out - buf);
-		total_read += stream->next_out - buf;
-	}
-
-	if (status != Z_STREAM_END) {
-		error(_("corrupt loose object '%s'"), oid_to_hex(expected_oid));
-		git_hash_discard(&c);
-		return -1;
-	}
-	if (stream->avail_in) {
-		error(_("garbage at end of loose object '%s'"),
-		      oid_to_hex(expected_oid));
-		git_hash_discard(&c);
-		return -1;
-	}
-
-	git_hash_final_oid(&real_oid, &c);
-	if (!oideq(expected_oid, &real_oid)) {
-		error(_("hash mismatch for %s (expected %s)"), path,
-		      oid_to_hex(expected_oid));
-		return -1;
-	}
-
-	return 0;
-}
-
-int read_loose_object(struct repository *repo,
-		      const char *path,
-		      const struct object_id *expected_oid,
-		      struct object_id *real_oid,
-		      void **contents,
-		      struct object_info *oi)
-{
-	int ret = -1;
-	int fd;
-	void *map = NULL;
-	unsigned long mapsize;
-	git_zstream stream;
-	char hdr[MAX_HEADER_LEN];
-	size_t *size = oi->sizep;
-
-	fd = git_open(path);
-	if (fd >= 0)
-		map = map_fd(fd, path, &mapsize);
-	if (!map) {
-		error_errno(_("unable to mmap %s"), path);
-		goto out;
-	}
-
-	if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) != ULHR_OK) {
-		error(_("unable to unpack header of %s"), path);
-		goto out_inflate;
-	}
-
-	if (parse_loose_header(hdr, oi) < 0) {
-		error(_("unable to parse header of %s"), path);
-		goto out_inflate;
-	}
-
-	if (*oi->typep < 0) {
-		error(_("unable to parse type from header '%s' of %s"),
-		      hdr, path);
-		goto out_inflate;
-	}
-
-	if (*oi->typep == OBJ_BLOB &&
-	    *size > repo_settings_get_big_file_threshold(repo)) {
-		if (check_stream_oid(&stream, hdr, *size, path, expected_oid,
-				     repo->hash_algo) < 0)
-			goto out_inflate;
-	} else {
-		*contents = unpack_loose_rest(&stream, hdr, *size, expected_oid);
-		if (!*contents) {
-			error(_("unable to unpack contents of %s"), path);
-			goto out_inflate;
-		}
-		hash_object_file(repo->hash_algo,
-				 *contents, *size,
-				 *oi->typep, real_oid);
-		if (!oideq(expected_oid, real_oid))
-			goto out_inflate;
-	}
-
-	ret = 0; /* everything checks out */
-
-out_inflate:
-	git_inflate_end(&stream);
-out:
-	if (map)
-		munmap(map, mapsize);
-	return ret;
-}
-
 static int write_pack_lockfile(struct odb_source *source, const char *pack_name,
 			       const char *keep_msg, struct tempfile **out)
 {
diff --git a/object-file.h b/object-file.h
index f44758c4f8b..65ab44542b0 100644
--- a/object-file.h
+++ b/object-file.h
@@ -21,7 +21,6 @@ enum {
 int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
 int index_path(struct index_state *istate, struct object_id *oid, const char *path, struct stat *st, unsigned flags);
 
-struct object_info;
 struct odb_source;
 
 /*
@@ -122,50 +121,6 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 int check_and_freshen_file(const char *fn, int freshen,
 			   const time_t *mtime);
 
-/*
- * Open the loose object at path, check its hash, and return the contents,
- * use the "oi" argument to assert things about the object, or e.g. populate its
- * type, and size. If the object is a blob, then "contents" may return NULL,
- * to allow streaming of large blobs.
- *
- * Returns 0 on success, negative on error (details may be written to stderr).
- */
-int read_loose_object(struct repository *repo,
-		      const char *path,
-		      const struct object_id *expected_oid,
-		      struct object_id *real_oid,
-		      void **contents,
-		      struct object_info *oi);
-
-enum unpack_loose_header_result {
-	ULHR_OK,
-	ULHR_BAD,
-	ULHR_TOO_LONG,
-};
-
-/**
- * unpack_loose_header() initializes the data stream needed to unpack
- * a loose object header.
- *
- * Returns:
- *
- * - ULHR_OK on success
- * - ULHR_BAD on error
- * - ULHR_TOO_LONG if the header was too long
- *
- * It will only parse up to MAX_HEADER_LEN bytes.
- */
-enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
-						    unsigned char *map,
-						    unsigned long mapsize,
-						    void *buffer,
-						    unsigned long bufsiz);
-void *unpack_loose_rest(git_zstream *stream,
-			void *buffer, unsigned long size,
-			const struct object_id *oid);
-
-int parse_loose_header(const char *hdr, struct object_info *oi);
-
 struct odb_transaction;
 
 /*
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c352..96fc2ed213f 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -64,6 +64,162 @@ static int quick_has_loose(struct odb_source_loose *loose,
 	return !!oidtree_contains(odb_source_loose_cache(loose, oid), oid);
 }
 
+enum unpack_loose_header_result {
+	ULHR_OK,
+	ULHR_BAD,
+	ULHR_TOO_LONG,
+};
+
+static enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
+							   unsigned char *map,
+							   unsigned long mapsize,
+							   void *buffer,
+							   unsigned long bufsiz)
+{
+	int status;
+
+	/* Get the data stream */
+	memset(stream, 0, sizeof(*stream));
+	stream->next_in = map;
+	stream->avail_in = mapsize;
+	stream->next_out = buffer;
+	stream->avail_out = bufsiz;
+
+	git_inflate_init(stream);
+	obj_read_unlock();
+	status = git_inflate(stream, 0);
+	obj_read_lock();
+	if (status != Z_OK && status != Z_STREAM_END)
+		return ULHR_BAD;
+
+	/*
+	 * Check if entire header is unpacked in the first iteration.
+	 */
+	if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
+		return ULHR_OK;
+
+	/*
+	 * We have a header longer than MAX_HEADER_LEN.
+	 */
+	return ULHR_TOO_LONG;
+}
+
+static void *unpack_loose_rest(git_zstream *stream,
+			       void *buffer, unsigned long size,
+			       const struct object_id *oid)
+{
+	size_t bytes = strlen(buffer) + 1, n;
+	unsigned char *buf = xmallocz(size);
+	int status = Z_OK;
+
+	n = stream->total_out - bytes;
+	if (n > size)
+		n = size;
+	memcpy(buf, (char *) buffer + bytes, n);
+	bytes = n;
+	if (bytes <= size) {
+		/*
+		 * The above condition must be (bytes <= size), not
+		 * (bytes < size).  In other words, even though we
+		 * expect no more output and set avail_out to zero,
+		 * the input zlib stream may have bytes that express
+		 * "this concludes the stream", and we *do* want to
+		 * eat that input.
+		 *
+		 * Otherwise we would not be able to test that we
+		 * consumed all the input to reach the expected size;
+		 * we also want to check that zlib tells us that all
+		 * went well with status == Z_STREAM_END at the end.
+		 */
+		stream->next_out = buf + bytes;
+		stream->avail_out = size - bytes;
+		while (status == Z_OK) {
+			obj_read_unlock();
+			status = git_inflate(stream, Z_FINISH);
+			obj_read_lock();
+		}
+	}
+
+	if (status != Z_STREAM_END) {
+		error(_("corrupt loose object '%s'"), oid_to_hex(oid));
+		FREE_AND_NULL(buf);
+	} else if (stream->avail_in) {
+		error(_("garbage at end of loose object '%s'"),
+		      oid_to_hex(oid));
+		FREE_AND_NULL(buf);
+	}
+
+	return buf;
+}
+
+/*
+ * parse_loose_header() parses the starting "<type> <len>\0" of an
+ * object. If it doesn't follow that format -1 is returned. To check
+ * the validity of the <type> populate the "typep" in the "struct
+ * object_info". It will be OBJ_BAD if the object type is unknown. The
+ * parsed <len> can be retrieved via "oi->sizep", and from there
+ * passed to unpack_loose_rest().
+ *
+ * We used to just use "sscanf()", but that's actually way
+ * too permissive for what we want to check. So do an anal
+ * object header parse by hand.
+ */
+static int parse_loose_header(const char *hdr, struct object_info *oi)
+{
+	const char *type_buf = hdr;
+	size_t size;
+	int type, type_len = 0;
+
+	/*
+	 * The type can be of any size but is followed by
+	 * a space.
+	 */
+	for (;;) {
+		char c = *hdr++;
+		if (!c)
+			return -1;
+		if (c == ' ')
+			break;
+		type_len++;
+	}
+
+	type = type_from_string_gently(type_buf, type_len, 1);
+	if (oi->typep)
+		*oi->typep = type;
+
+	/*
+	 * The length must follow immediately, and be in canonical
+	 * decimal format (ie "010" is not valid).
+	 */
+	size = *hdr++ - '0';
+	if (size > 9)
+		return -1;
+	if (size) {
+		for (;;) {
+			unsigned long c = *hdr - '0';
+			if (c > 9)
+				break;
+			hdr++;
+			size = st_add(st_mult(size, 10), c);
+		}
+	}
+
+	if (oi->sizep)
+		*oi->sizep = size;
+
+	/*
+	 * The length must be followed by a zero byte
+	 */
+	if (*hdr)
+		return -1;
+
+	/*
+	 * The format is valid, but the type may still be bogus. The
+	 * Caller needs to check its oi->typep.
+	 */
+	return 0;
+}
+
 static int read_object_info_from_path(struct odb_source_loose *loose,
 				      const char *path,
 				      const struct object_id *oid,
@@ -248,6 +404,29 @@ static int open_loose_object(struct odb_source_loose *loose,
 	return -1;
 }
 
+/*
+ * Map and close the given loose object fd. The path argument is used for
+ * error reporting.
+ */
+static void *map_fd(int fd, const char *path, unsigned long *size)
+{
+	void *map = NULL;
+	struct stat st;
+
+	if (!fstat(fd, &st)) {
+		*size = xsize_t(st.st_size);
+		if (!*size) {
+			/* mmap() is forbidden on empty files */
+			error(_("object file %s is empty"), path);
+			close(fd);
+			return NULL;
+		}
+		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
+	}
+	close(fd);
+	return map;
+}
+
 static void *odb_source_loose_map_object(struct odb_source_loose *loose,
 					 const struct object_id *oid,
 					 unsigned long *size)
@@ -1015,6 +1194,133 @@ static void odb_source_loose_free(struct odb_source *source)
 	free(loose);
 }
 
+static int check_stream_oid(git_zstream *stream,
+			    const char *hdr,
+			    unsigned long size,
+			    const char *path,
+			    const struct object_id *expected_oid,
+			    const struct git_hash_algo *algop)
+{
+	struct git_hash_ctx c;
+	struct object_id real_oid;
+	unsigned char buf[4096];
+	unsigned long total_read;
+	int status = Z_OK;
+
+	git_hash_init(&c, algop);
+	git_hash_update(&c, hdr, stream->total_out);
+
+	/*
+	 * We already read some bytes into hdr, but the ones up to the NUL
+	 * do not count against the object's content size.
+	 */
+	total_read = stream->total_out - strlen(hdr) - 1;
+
+	/*
+	 * This size comparison must be "<=" to read the final zlib packets;
+	 * see the comment in unpack_loose_rest for details.
+	 */
+	while (total_read <= size &&
+	       (status == Z_OK ||
+		(status == Z_BUF_ERROR && !stream->avail_out))) {
+		stream->next_out = buf;
+		stream->avail_out = sizeof(buf);
+		if (size - total_read < stream->avail_out)
+			stream->avail_out = size - total_read;
+		status = git_inflate(stream, Z_FINISH);
+		git_hash_update(&c, buf, stream->next_out - buf);
+		total_read += stream->next_out - buf;
+	}
+
+	if (status != Z_STREAM_END) {
+		error(_("corrupt loose object '%s'"), oid_to_hex(expected_oid));
+		git_hash_discard(&c);
+		return -1;
+	}
+	if (stream->avail_in) {
+		error(_("garbage at end of loose object '%s'"),
+		      oid_to_hex(expected_oid));
+		git_hash_discard(&c);
+		return -1;
+	}
+
+	git_hash_final_oid(&real_oid, &c);
+	if (!oideq(expected_oid, &real_oid)) {
+		error(_("hash mismatch for %s (expected %s)"), path,
+		      oid_to_hex(expected_oid));
+		return -1;
+	}
+
+	return 0;
+}
+
+static int read_loose_object(struct repository *repo,
+			     const char *path,
+			     const struct object_id *expected_oid,
+			     struct object_id *real_oid,
+			     void **contents,
+			     struct object_info *oi)
+{
+	int ret = -1;
+	int fd;
+	void *map = NULL;
+	unsigned long mapsize;
+	git_zstream stream;
+	char hdr[MAX_HEADER_LEN];
+	size_t *size = oi->sizep;
+
+	fd = git_open(path);
+	if (fd >= 0)
+		map = map_fd(fd, path, &mapsize);
+	if (!map) {
+		error_errno(_("unable to mmap %s"), path);
+		goto out;
+	}
+
+	if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) != ULHR_OK) {
+		error(_("unable to unpack header of %s"), path);
+		goto out_inflate;
+	}
+
+	if (parse_loose_header(hdr, oi) < 0) {
+		error(_("unable to parse header of %s"), path);
+		goto out_inflate;
+	}
+
+	if (*oi->typep < 0) {
+		error(_("unable to parse type from header '%s' of %s"),
+		      hdr, path);
+		goto out_inflate;
+	}
+
+	if (*oi->typep == OBJ_BLOB &&
+	    *size > repo_settings_get_big_file_threshold(repo)) {
+		if (check_stream_oid(&stream, hdr, *size, path, expected_oid,
+				     repo->hash_algo) < 0)
+			goto out_inflate;
+	} else {
+		*contents = unpack_loose_rest(&stream, hdr, *size, expected_oid);
+		if (!*contents) {
+			error(_("unable to unpack contents of %s"), path);
+			goto out_inflate;
+		}
+		hash_object_file(repo->hash_algo,
+				 *contents, *size,
+				 *oi->typep, real_oid);
+		if (!oideq(expected_oid, real_oid))
+			goto out_inflate;
+	}
+
+	ret = 0; /* everything checks out */
+
+out_inflate:
+	git_inflate_end(&stream);
+out:
+	if (map)
+		munmap(map, mapsize);
+	return ret;
+}
+
 struct fsck_loose_data {
 	struct odb_source_loose *source;
 	struct odb_fsck_options *opts;

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 1/6] object-file: move logic to read loose objects into backend

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> The logic to read loose objects is still contained in "object-file.c"
> even though it's an implementation detail of the "loose" backend, and
> that backend is nowadays also the only remaining caller.
> 
> Move the logic around and remove the accompanying declarations.
> 

The move looks justified as mentioned in the commit message.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   object-file.c      | 300 ---------------------------------------------------

All functions moved from object-file,c are properly moved over to 
odb/source-loose.c and they are marked static which is also appropriate.

>   object-file.h      |  45 --------
>   odb/source-loose.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 306 insertions(+), 345 deletions(-)

> diff --git a/object-file.h b/object-file.h
> index f44758c4f8b..65ab44542b0 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -21,7 +21,6 @@ enum {
>   int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
>   int index_path(struct index_state *istate, struct object_id *oid, const char *path, struct stat *st, unsigned flags);
>   
> -struct object_info;
>   struct odb_source;
>   
>   /*
> @@ -122,50 +121,6 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
>   int check_and_freshen_file(const char *fn, int freshen,
>   			   const time_t *mtime);
>   
> -/*
> - * Open the loose object at path, check its hash, and return the contents,
> - * use the "oi" argument to assert things about the object, or e.g. populate its
> - * type, and size. If the object is a blob, then "contents" may return NULL,
> - * to allow streaming of large blobs.
> - *
> - * Returns 0 on success, negative on error (details may be written to stderr).
> - */

We're losing these documentation comments during the move. Can we retain 
the same above the static functions in odb/source-loose.c?

Rest of the patch looks good to me.

-- 
Sivaraam

Re: [PATCH 1/6] object-file: move logic to read loose objects into backend

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 06:40:59PM +0530, Kaartic Sivaraam wrote:
> On 11/2/26 16:50, Patrick Steinhardt wrote:
> > diff --git a/object-file.h b/object-file.h
> > index f44758c4f8b..65ab44542b0 100644
> > --- a/object-file.h
> > +++ b/object-file.h
> > @@ -122,50 +121,6 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
> >   int check_and_freshen_file(const char *fn, int freshen,
> >   			   const time_t *mtime);
> > -/*
> > - * Open the loose object at path, check its hash, and return the contents,
> > - * use the "oi" argument to assert things about the object, or e.g. populate its
> > - * type, and size. If the object is a blob, then "contents" may return NULL,
> > - * to allow streaming of large blobs.
> > - *
> > - * Returns 0 on success, negative on error (details may be written to stderr).
> > - */
> 
> We're losing these documentation comments during the move. Can we retain the
> same above the static functions in odb/source-loose.c?

Sure, can do.

Patrick

[PATCH 2/6] object-file: move logic to derive loose object paths into backend

[Patrick Steinhardt]

The logic to derive the filesystem path of a loose object is contained
in "object-file.c" even though it's an implementation detail of the
"loose" backend. Move the logic into the "loose" backend accordingly.
While at it, inline the call to `fill_loose_path()`.

Ideally, we'd be able to remove the externally-visible declaration. But
unfortunately, we still require access to that function for use in the
dumb transport so that it can derive the target path for downloaded
objects. This will need to get abstracted eventually, but for now we
simply continue to expose the function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c      | 25 -------------------------
 object-file.h      |  8 --------
 odb/source-loose.c | 21 +++++++++++++++++++++
 odb/source-loose.h |  8 ++++++++
 4 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/object-file.c b/object-file.c
index c1b78a25c9f..ab919f03fef 100644
--- a/object-file.c
+++ b/object-file.c
@@ -43,31 +43,6 @@ static int get_conv_flags(unsigned flags)
 		return 0;
 }
 
-static void fill_loose_path(struct strbuf *buf,
-			    const struct object_id *oid,
-			    const struct git_hash_algo *algop)
-{
-	for (size_t i = 0; i < algop->rawsz; i++) {
-		static char hex[] = "0123456789abcdef";
-		unsigned int val = oid->hash[i];
-		strbuf_addch(buf, hex[val >> 4]);
-		strbuf_addch(buf, hex[val & 0xf]);
-		if (!i)
-			strbuf_addch(buf, '/');
-	}
-}
-
-const char *odb_loose_path(struct odb_source_loose *loose,
-			   struct strbuf *buf,
-			   const struct object_id *oid)
-{
-	strbuf_reset(buf);
-	strbuf_addstr(buf, loose->base.path);
-	strbuf_addch(buf, '/');
-	fill_loose_path(buf, oid, loose->base.odb->repo->hash_algo);
-	return buf->buf;
-}
-
 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
 static int freshen_file(const char *fn, const time_t *mtime)
 {
diff --git a/object-file.h b/object-file.h
index 65ab44542b0..3cfcaefa420 100644
--- a/object-file.h
+++ b/object-file.h
@@ -23,14 +23,6 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa
 
 struct odb_source;
 
-/*
- * Put in `buf` the name of the file in the local object database that
- * would be used to store a loose object with the specified oid.
- */
-const char *odb_loose_path(struct odb_source_loose *source,
-			   struct strbuf *buf,
-			   const struct object_id *oid);
-
 /*
  * Iterate over the files in the loose-object parts of the object
  * directory "path", triggering the following callbacks:
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 96fc2ed213f..0f29162d752 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -18,6 +18,27 @@
 #include "tempfile.h"
 #include "write-or-die.h"
 
+const char *odb_loose_path(struct odb_source_loose *loose,
+			   struct strbuf *buf,
+			   const struct object_id *oid)
+{
+	static const char hex[] = "0123456789abcdef";
+
+	strbuf_reset(buf);
+	strbuf_addstr(buf, loose->base.path);
+	strbuf_addch(buf, '/');
+
+	for (size_t i = 0; i < loose->base.odb->repo->hash_algo->rawsz; i++) {
+		unsigned int val = oid->hash[i];
+		strbuf_addch(buf, hex[val >> 4]);
+		strbuf_addch(buf, hex[val & 0xf]);
+		if (!i)
+			strbuf_addch(buf, '/');
+	}
+
+	return buf->buf;
+}
+
 static int append_loose_object(const struct object_id *oid,
 			       const char *path UNUSED,
 			       void *data)
diff --git a/odb/source-loose.h b/odb/source-loose.h
index a6ae808662b..8d4cdac0d50 100644
--- a/odb/source-loose.h
+++ b/odb/source-loose.h
@@ -49,4 +49,12 @@ static inline struct odb_source_loose *odb_source_loose_downcast(struct odb_sour
 	return container_of(source, struct odb_source_loose, base);
 }
 
+/*
+ * Put in `buf` the name of the file in the local object database that
+ * would be used to store a loose object with the specified oid.
+ */
+const char *odb_loose_path(struct odb_source_loose *source,
+			   struct strbuf *buf,
+			   const struct object_id *oid);
+
 #endif

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 2/6] object-file: move logic to derive loose object paths into backend

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> The logic to derive the filesystem path of a loose object is contained
> in "object-file.c" even though it's an implementation detail of the
> "loose" backend. Move the logic into the "loose" backend accordingly.
> While at it, inline the call to `fill_loose_path()`.
> 
> Ideally, we'd be able to remove the externally-visible declaration. But
> unfortunately, we still require access to that function for use in the
> dumb transport so that it can derive the target path for downloaded
> objects. This will need to get abstracted eventually, but for now we
> simply continue to expose the function.
>

This is well explained.
> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index 96fc2ed213f..0f29162d752 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -18,6 +18,27 @@
>   #include "tempfile.h"
>   #include "write-or-die.h"
>   
> +const char *odb_loose_path(struct odb_source_loose *loose,
> +			   struct strbuf *buf,
> +			   const struct object_id *oid)
> +{
> +	static const char hex[] = "0123456789abcdef";
> +

The hex[] gained a const while moving but that looks obviously correct.

Rest of the move looks good to me. We didn't lose anything. One thing I 
noticed is that http-walker.c no longer needs the object-file.c include 
now. So, we could also include the following in this patch:

diff --git a/http-walker.c b/http-walker.c
index 13730a2784..05c0262810 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -9,7 +9,6 @@
  #include "list.h"
  #include "transport.h"
  #include "packfile.h"
-#include "object-file.h"
  #include "odb.h"
  #include "odb/source-loose.h"

-- 
Sivaraam

Re: [PATCH 2/6] object-file: move logic to derive loose object paths into backend

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 06:58:34PM +0530, Kaartic Sivaraam wrote:
> On 11/2/26 16:50, Patrick Steinhardt wrote:
> > diff --git a/odb/source-loose.c b/odb/source-loose.c
> > index 96fc2ed213f..0f29162d752 100644
> > --- a/odb/source-loose.c
> > +++ b/odb/source-loose.c
> > @@ -18,6 +18,27 @@
> >   #include "tempfile.h"
> >   #include "write-or-die.h"
> > +const char *odb_loose_path(struct odb_source_loose *loose,
> > +			   struct strbuf *buf,
> > +			   const struct object_id *oid)
> > +{
> > +	static const char hex[] = "0123456789abcdef";
> > +
> 
> The hex[] gained a const while moving but that looks obviously correct.
> 
> Rest of the move looks good to me. We didn't lose anything. One thing I
> noticed is that http-walker.c no longer needs the object-file.c include now.
> So, we could also include the following in this patch:
> 
> diff --git a/http-walker.c b/http-walker.c
> index 13730a2784..05c0262810 100644
> --- a/http-walker.c
> +++ b/http-walker.c
> @@ -9,7 +9,6 @@
>  #include "list.h"
>  #include "transport.h"
>  #include "packfile.h"
> -#include "object-file.h"
>  #include "odb.h"
>  #include "odb/source-loose.h"

It still needs that header at this step though. But it doesn't need it
anymore after patch 5, so let me fold that in there.

Patrick

[PATCH 3/6] object-file: move iteration over loose files into backend

[Patrick Steinhardt]

The logic to iterate over files part of the "loose" backend is contained
in "object-file.c" even though it's an implementation detail of the
backend itself. Move the logic into the backend accordingly.

Note that the function is a bit on the weird side: it accepts a generic
`struct odb_source` as input and doesn't cast it down to its backend at
all. This is because all it requires access to is the `path` member of
the source, and from thereon it knows how to iterate the subdirectories
of the "loose" backend.

We should adapt this function to accept `struct odb_source_loose` to
properly reflect that it expects a path to a loose object database. But
that would require changes to a bunch of callers without much benefit.
So for now, the function signature is left as-is. But eventually we may
want to change it once we have reworked a couple of its callers.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/count-objects.c |  1 +
 builtin/gc.c            |  1 +
 builtin/prune.c         |  1 +
 object-file.c           | 92 -------------------------------------------------
 object-file.h           | 44 -----------------------
 odb/source-loose.c      | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
 odb/source-loose.h      | 33 ++++++++++++++++++
 prune-packed.c          |  1 +
 8 files changed, 129 insertions(+), 136 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 3f7b20fde53..aae040567e3 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -14,6 +14,7 @@
 #include "quote.h"
 #include "packfile.h"
 #include "object-file.h"
+#include "odb/source-loose.h"
 
 static unsigned long garbage;
 static off_t size_garbage;
diff --git a/builtin/gc.c b/builtin/gc.c
index 81cecf6b6c3..d3ba6f0c9e9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -31,6 +31,7 @@
 #include "packfile.h"
 #include "object-file.h"
 #include "odb.h"
+#include "odb/source-loose.h"
 #include "path.h"
 #include "reflog.h"
 #include "rerere.h"
diff --git a/builtin/prune.c b/builtin/prune.c
index 97f4c1418bc..af2a547d931 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -17,6 +17,7 @@
 #include "object-file.h"
 #include "object-name.h"
 #include "odb.h"
+#include "odb/source-loose.h"
 #include "shallow.h"
 
 static const char * const prune_usage[] = {
diff --git a/object-file.c b/object-file.c
index ab919f03fef..d3ddcbf309c 100644
--- a/object-file.c
+++ b/object-file.c
@@ -12,7 +12,6 @@
 #include "git-compat-util.h"
 #include "config.h"
 #include "convert.h"
-#include "dir.h"
 #include "environment.h"
 #include "fsck.h"
 #include "gettext.h"
@@ -838,97 +837,6 @@ int read_pack_header(int fd, struct pack_header *header)
 	return 0;
 }
 
-int for_each_file_in_obj_subdir(unsigned int subdir_nr,
-				struct strbuf *path,
-				const struct git_hash_algo *algop,
-				each_loose_object_fn obj_cb,
-				each_loose_cruft_fn cruft_cb,
-				each_loose_subdir_fn subdir_cb,
-				void *data)
-{
-	size_t origlen, baselen;
-	DIR *dir;
-	struct dirent *de;
-	int r = 0;
-	struct object_id oid;
-
-	if (subdir_nr > 0xff)
-		BUG("invalid loose object subdirectory: %x", subdir_nr);
-
-	origlen = path->len;
-	strbuf_complete(path, '/');
-	strbuf_addf(path, "%02x", subdir_nr);
-
-	dir = opendir(path->buf);
-	if (!dir) {
-		if (errno != ENOENT)
-			r = error_errno(_("unable to open %s"), path->buf);
-		strbuf_setlen(path, origlen);
-		return r;
-	}
-
-	oid.hash[0] = subdir_nr;
-	strbuf_addch(path, '/');
-	baselen = path->len;
-
-	while ((de = readdir_skip_dot_and_dotdot(dir))) {
-		size_t namelen;
-
-		namelen = strlen(de->d_name);
-		strbuf_setlen(path, baselen);
-		strbuf_add(path, de->d_name, namelen);
-		if (namelen == algop->hexsz - 2 &&
-		    !hex_to_bytes(oid.hash + 1, de->d_name,
-				  algop->rawsz - 1)) {
-			oid_set_algo(&oid, algop);
-			memset(oid.hash + algop->rawsz, 0,
-			       GIT_MAX_RAWSZ - algop->rawsz);
-			if (obj_cb) {
-				r = obj_cb(&oid, path->buf, data);
-				if (r)
-					break;
-			}
-			continue;
-		}
-
-		if (cruft_cb) {
-			r = cruft_cb(de->d_name, path->buf, data);
-			if (r)
-				break;
-		}
-	}
-	closedir(dir);
-
-	strbuf_setlen(path, baselen - 1);
-	if (!r && subdir_cb)
-		r = subdir_cb(subdir_nr, path->buf, data);
-
-	strbuf_setlen(path, origlen);
-
-	return r;
-}
-
-int for_each_loose_file_in_source(struct odb_source *source,
-				  each_loose_object_fn obj_cb,
-				  each_loose_cruft_fn cruft_cb,
-				  each_loose_subdir_fn subdir_cb,
-				  void *data)
-{
-	struct strbuf buf = STRBUF_INIT;
-	int r;
-
-	strbuf_addstr(&buf, source->path);
-	for (int i = 0; i < 256; i++) {
-		r = for_each_file_in_obj_subdir(i, &buf, source->odb->repo->hash_algo,
-						obj_cb, cruft_cb, subdir_cb, data);
-		if (r)
-			break;
-	}
-
-	strbuf_release(&buf);
-	return r;
-}
-
 static int write_pack_lockfile(struct odb_source *source, const char *pack_name,
 			       const char *keep_msg, struct tempfile **out)
 {
diff --git a/object-file.h b/object-file.h
index 3cfcaefa420..83d3f38a540 100644
--- a/object-file.h
+++ b/object-file.h
@@ -23,50 +23,6 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa
 
 struct odb_source;
 
-/*
- * Iterate over the files in the loose-object parts of the object
- * directory "path", triggering the following callbacks:
- *
- *  - loose_object is called for each loose object we find.
- *
- *  - loose_cruft is called for any files that do not appear to be
- *    loose objects. Note that we only look in the loose object
- *    directories "objects/[0-9a-f]{2}/", so we will not report
- *    "objects/foobar" as cruft.
- *
- *  - loose_subdir is called for each top-level hashed subdirectory
- *    of the object directory (e.g., "$OBJDIR/f0"). It is called
- *    after the objects in the directory are processed.
- *
- * Any callback that is NULL will be ignored. Callbacks returning non-zero
- * will end the iteration.
- *
- * In the "buf" variant, "path" is a strbuf which will also be used as a
- * scratch buffer, but restored to its original contents before
- * the function returns.
- */
-typedef int each_loose_object_fn(const struct object_id *oid,
-				 const char *path,
-				 void *data);
-typedef int each_loose_cruft_fn(const char *basename,
-				const char *path,
-				void *data);
-typedef int each_loose_subdir_fn(unsigned int nr,
-				 const char *path,
-				 void *data);
-int for_each_loose_file_in_source(struct odb_source *source,
-				  each_loose_object_fn obj_cb,
-				  each_loose_cruft_fn cruft_cb,
-				  each_loose_subdir_fn subdir_cb,
-				  void *data);
-int for_each_file_in_obj_subdir(unsigned int subdir_nr,
-				struct strbuf *path,
-				const struct git_hash_algo *algop,
-				each_loose_object_fn obj_cb,
-				each_loose_cruft_fn cruft_cb,
-				each_loose_subdir_fn subdir_cb,
-				void *data);
-
 /**
  * format_object_header() is a thin wrapper around s xsnprintf() that
  * writes the initial "<type> <obj-len>" part of the loose object
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 0f29162d752..a90268e200a 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -1,6 +1,7 @@
 #include "git-compat-util.h"
 #include "abspath.h"
 #include "chdir-notify.h"
+#include "dir.h"
 #include "gettext.h"
 #include "hex.h"
 #include "loose.h"
@@ -39,6 +40,97 @@ const char *odb_loose_path(struct odb_source_loose *loose,
 	return buf->buf;
 }
 
+static int for_each_file_in_obj_subdir(unsigned int subdir_nr,
+				       struct strbuf *path,
+				       const struct git_hash_algo *algop,
+				       each_loose_object_fn obj_cb,
+				       each_loose_cruft_fn cruft_cb,
+				       each_loose_subdir_fn subdir_cb,
+				       void *data)
+{
+	size_t origlen, baselen;
+	DIR *dir;
+	struct dirent *de;
+	int r = 0;
+	struct object_id oid;
+
+	if (subdir_nr > 0xff)
+		BUG("invalid loose object subdirectory: %x", subdir_nr);
+
+	origlen = path->len;
+	strbuf_complete(path, '/');
+	strbuf_addf(path, "%02x", subdir_nr);
+
+	dir = opendir(path->buf);
+	if (!dir) {
+		if (errno != ENOENT)
+			r = error_errno(_("unable to open %s"), path->buf);
+		strbuf_setlen(path, origlen);
+		return r;
+	}
+
+	oid.hash[0] = subdir_nr;
+	strbuf_addch(path, '/');
+	baselen = path->len;
+
+	while ((de = readdir_skip_dot_and_dotdot(dir))) {
+		size_t namelen;
+
+		namelen = strlen(de->d_name);
+		strbuf_setlen(path, baselen);
+		strbuf_add(path, de->d_name, namelen);
+		if (namelen == algop->hexsz - 2 &&
+		    !hex_to_bytes(oid.hash + 1, de->d_name,
+				  algop->rawsz - 1)) {
+			oid_set_algo(&oid, algop);
+			memset(oid.hash + algop->rawsz, 0,
+			       GIT_MAX_RAWSZ - algop->rawsz);
+			if (obj_cb) {
+				r = obj_cb(&oid, path->buf, data);
+				if (r)
+					break;
+			}
+			continue;
+		}
+
+		if (cruft_cb) {
+			r = cruft_cb(de->d_name, path->buf, data);
+			if (r)
+				break;
+		}
+	}
+	closedir(dir);
+
+	strbuf_setlen(path, baselen - 1);
+	if (!r && subdir_cb)
+		r = subdir_cb(subdir_nr, path->buf, data);
+
+	strbuf_setlen(path, origlen);
+
+	return r;
+}
+
+int for_each_loose_file_in_source(struct odb_source *source,
+				  each_loose_object_fn obj_cb,
+				  each_loose_cruft_fn cruft_cb,
+				  each_loose_subdir_fn subdir_cb,
+				  void *data)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int r;
+
+	strbuf_addstr(&buf, source->path);
+	for (int i = 0; i < 256; i++) {
+		r = for_each_file_in_obj_subdir(i, &buf, source->odb->repo->hash_algo,
+						obj_cb, cruft_cb, subdir_cb, data);
+		if (r)
+			break;
+	}
+
+	strbuf_release(&buf);
+	return r;
+}
+
 static int append_loose_object(const struct object_id *oid,
 			       const char *path UNUSED,
 			       void *data)
diff --git a/odb/source-loose.h b/odb/source-loose.h
index 8d4cdac0d50..20f6ec7fa08 100644
--- a/odb/source-loose.h
+++ b/odb/source-loose.h
@@ -57,4 +57,37 @@ const char *odb_loose_path(struct odb_source_loose *source,
 			   struct strbuf *buf,
 			   const struct object_id *oid);
 
+/*
+ * Iterate over the files in the loose-object parts of the object
+ * directory "path", triggering the following callbacks:
+ *
+ *  - loose_object is called for each loose object we find.
+ *
+ *  - loose_cruft is called for any files that do not appear to be
+ *    loose objects. Note that we only look in the loose object
+ *    directories "objects/[0-9a-f]{2}/", so we will not report
+ *    "objects/foobar" as cruft.
+ *
+ *  - loose_subdir is called for each top-level hashed subdirectory
+ *    of the object directory (e.g., "$OBJDIR/f0"). It is called
+ *    after the objects in the directory are processed.
+ *
+ * Any callback that is NULL will be ignored. Callbacks returning non-zero
+ * will end the iteration.
+ */
+typedef int each_loose_object_fn(const struct object_id *oid,
+				 const char *path,
+				 void *data);
+typedef int each_loose_cruft_fn(const char *basename,
+				const char *path,
+				void *data);
+typedef int each_loose_subdir_fn(unsigned int nr,
+				 const char *path,
+				 void *data);
+int for_each_loose_file_in_source(struct odb_source *source,
+				  each_loose_object_fn obj_cb,
+				  each_loose_cruft_fn cruft_cb,
+				  each_loose_subdir_fn subdir_cb,
+				  void *data);
+
 #endif
diff --git a/prune-packed.c b/prune-packed.c
index b6749b8d132..d0a8a062b97 100644
--- a/prune-packed.c
+++ b/prune-packed.c
@@ -3,6 +3,7 @@
 #include "git-compat-util.h"
 #include "gettext.h"
 #include "object-file.h"
+#include "odb/source-loose.h"
 #include "packfile.h"
 #include "progress.h"
 #include "prune-packed.h"

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 3/6] object-file: move iteration over loose files into backend

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> The logic to iterate over files part of the "loose" backend is contained
> in "object-file.c" even though it's an implementation detail of the
> backend itself. Move the logic into the backend accordingly.
> 
> Note that the function is a bit on the weird side: it accepts a generic
> `struct odb_source` as input and doesn't cast it down to its backend at
> all. This is because all it requires access to is the `path` member of
> the source, and from thereon it knows how to iterate the subdirectories
> of the "loose" backend.
> 
> We should adapt this function to accept `struct odb_source_loose` to
> properly reflect that it expects a path to a loose object database. But
> that would require changes to a bunch of callers without much benefit.
> So for now, the function signature is left as-is. But eventually we may
> want to change it once we have reworked a couple of its callers.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   builtin/count-objects.c |  1 +
>   builtin/gc.c            |  1 +
>   builtin/prune.c         |  1 +
>   object-file.c           | 92 -------------------------------------------------
>   object-file.h           | 44 -----------------------
>   odb/source-loose.c      | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
>   odb/source-loose.h      | 33 ++++++++++++++++++
>   prune-packed.c          |  1 +
>   8 files changed, 129 insertions(+), 136 deletions(-)
> 
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 3f7b20fde53..aae040567e3 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -14,6 +14,7 @@
>   #include "quote.h"
>   #include "packfile.h"
>   #include "object-file.h"
> +#include "odb/source-loose.h"
>   

It seems like we can remove the include of object-file.h while at it.
>   static unsigned long garbage;
>   static off_t size_garbage;
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 81cecf6b6c3..d3ba6f0c9e9 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -31,6 +31,7 @@
>   #include "packfile.h"
>   #include "object-file.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   #include "path.h"
>   #include "reflog.h"
>   #include "rerere.h"
> diff --git a/builtin/prune.c b/builtin/prune.c
> index 97f4c1418bc..af2a547d931 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -17,6 +17,7 @@
>   #include "object-file.h"
>   #include "object-name.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   #include "shallow.h"
>   

Ditto for the above two.

>   
> +static int for_each_file_in_obj_subdir(unsigned int subdir_nr,
> +				       struct strbuf *path,
> +				       const struct git_hash_algo *algop,
> +				       each_loose_object_fn obj_cb,
> +				       each_loose_cruft_fn cruft_cb,
> +				       each_loose_subdir_fn subdir_cb,
> +				       void *data)

This was made 'static' during the move but that seems to make sense to me.

> diff --git a/prune-packed.c b/prune-packed.c
> index b6749b8d132..d0a8a062b97 100644
> --- a/prune-packed.c
> +++ b/prune-packed.c
> @@ -3,6 +3,7 @@
>   #include "git-compat-util.h"
>   #include "gettext.h"
>   #include "object-file.h"
> +#include "odb/source-loose.h"

This could also benefit from losing the include of object-file.h

Rest of the patch looks good to me.

-- 
Sivaraam

[PATCH 4/6] odb/source-loose: deduplicate logic to mmap objects

[Patrick Steinhardt]

The logic to mmap objects is duplicated across `map_fd()` and
`odb_source_loose_map_object()`. The only difference is that the former
takes an arbitrary path, whereas the latter derives the path from an
object ID.

Deduplicate the logic.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb/source-loose.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/odb/source-loose.c b/odb/source-loose.c
index a90268e200a..5cd679e802c 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -544,28 +544,11 @@ static void *odb_source_loose_map_object(struct odb_source_loose *loose,
 					 const struct object_id *oid,
 					 unsigned long *size)
 {
-	const char *p;
-	int fd = open_loose_object(loose, oid, &p);
-	void *map = NULL;
-	struct stat st;
-
+	const char *path;
+	int fd = open_loose_object(loose, oid, &path);
 	if (fd < 0)
 		return NULL;
-
-	if (!fstat(fd, &st)) {
-		*size = xsize_t(st.st_size);
-		if (!*size) {
-			/* mmap() is forbidden on empty files */
-			error(_("object file %s is empty"), p);
-			goto out;
-		}
-
-		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
-	}
-
-out:
-	close(fd);
-	return map;
+	return map_fd(fd, path, size);
 }
 
 struct odb_loose_read_stream {

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 4/6] odb/source-loose: deduplicate logic to mmap objects

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> The logic to mmap objects is duplicated across `map_fd()` and
> `odb_source_loose_map_object()`. The only difference is that the former
> takes an arbitrary path, whereas the latter derives the path from an
> object ID.
> 
> Deduplicate the logic.
> 

Makes sense.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   odb/source-loose.c | 23 +++--------------------
>   1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index a90268e200a..5cd679e802c 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -544,28 +544,11 @@ static void *odb_source_loose_map_object(struct odb_source_loose *loose,
>   					 const struct object_id *oid,
>   					 unsigned long *size)
>   {
> -	const char *p;
> -	int fd = open_loose_object(loose, oid, &p);
> -	void *map = NULL;
> -	struct stat st;
> -
> +	const char *path;
> +	int fd = open_loose_object(loose, oid, &path);
>   	if (fd < 0)
>   		return NULL;
> -
> -	if (!fstat(fd, &st)) {
> -		*size = xsize_t(st.st_size);
> -		if (!*size) {
> -			/* mmap() is forbidden on empty files */
> -			error(_("object file %s is empty"), p);
> -			goto out;
> -		}
> -
> -		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
> -	}
> -
> -out:
> -	close(fd);
> -	return map;
> +	return map_fd(fd, path, size);

I just noticed that map_fd could follow the goto pattern followed above 
and avoid the redundant close() call. Something like:

diff --git a/odb/source-loose.c b/odb/source-loose.c
index 737d583f03..4ace94084f 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -532,11 +532,12 @@ static void *map_fd(int fd, const char *path, 
unsigned long *size)
  		if (!*size) {
  			/* mmap() is forbidden on empty files */
  			error(_("object file %s is empty"), path);
-			close(fd);
-			return NULL;
+			goto out;
  		}
  		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
  	}
+
+out:
  	close(fd);
  	return map;
  }

Of course, this isn't really a show-stopper for this patch. Just another 
relevant cleanup I noticed.

-- 
Sivaraam

Re: [PATCH 4/6] odb/source-loose: deduplicate logic to mmap objects

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 07:45:58PM +0530, Kaartic Sivaraam wrote:
> On 11/2/26 16:50, Patrick Steinhardt wrote:
> > diff --git a/odb/source-loose.c b/odb/source-loose.c
> > index a90268e200a..5cd679e802c 100644
> > --- a/odb/source-loose.c
> > +++ b/odb/source-loose.c
> > @@ -544,28 +544,11 @@ static void *odb_source_loose_map_object(struct odb_source_loose *loose,
> >   					 const struct object_id *oid,
> >   					 unsigned long *size)
> >   {
> > -	const char *p;
> > -	int fd = open_loose_object(loose, oid, &p);
> > -	void *map = NULL;
> > -	struct stat st;
> > -
> > +	const char *path;
> > +	int fd = open_loose_object(loose, oid, &path);
> >   	if (fd < 0)
> >   		return NULL;
> > -
> > -	if (!fstat(fd, &st)) {
> > -		*size = xsize_t(st.st_size);
> > -		if (!*size) {
> > -			/* mmap() is forbidden on empty files */
> > -			error(_("object file %s is empty"), p);
> > -			goto out;
> > -		}
> > -
> > -		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
> > -	}
> > -
> > -out:
> > -	close(fd);
> > -	return map;
> > +	return map_fd(fd, path, size);
> 
> I just noticed that map_fd could follow the goto pattern followed above and
> avoid the redundant close() call. Something like:
> 
> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index 737d583f03..4ace94084f 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -532,11 +532,12 @@ static void *map_fd(int fd, const char *path, unsigned
> long *size)
>  		if (!*size) {
>  			/* mmap() is forbidden on empty files */
>  			error(_("object file %s is empty"), path);
> -			close(fd);
> -			return NULL;
> +			goto out;
>  		}
>  		map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
>  	}
> +
> +out:
>  	close(fd);
>  	return map;
>  }
> 
> Of course, this isn't really a show-stopper for this patch. Just another
> relevant cleanup I noticed.

True, but I'll skip this one as I don't otherwise touch `map_fd()`.

Patrick

[PATCH 5/6] object-file: stop including "odb/source-loose.h"

[Patrick Steinhardt]

We have now moved all functionality that relate to the loose backend
from "object-file.c" into the backend itself. Wrap up these refactorings
by not including "odb/source-loose.h" anymore and instead include the
file explicitly in all compilation units that require access to it.

Like this we can trivially identify exactly those files that still
depend on details of the loose backend itself and that thus require
further work to become truly backend-agnostic.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c | 1 +
 http-walker.c          | 1 +
 http.c                 | 1 +
 loose.c                | 1 +
 object-file.c          | 1 +
 object-file.h          | 2 +-
 setup.c                | 1 +
 7 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ca3a891dfbc..e53c69af31f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -34,6 +34,7 @@
 #include "object-file.h"
 #include "object-file-convert.h"
 #include "odb.h"
+#include "odb/source-loose.h"
 #include "odb/streaming.h"
 #include "replace-object.h"
 #include "dir.h"
diff --git a/http-walker.c b/http-walker.c
index 4f50dd9b8d0..13730a27847 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -11,6 +11,7 @@
 #include "packfile.h"
 #include "object-file.h"
 #include "odb.h"
+#include "odb/source-loose.h"
 
 struct alt_base {
 	char *base;
diff --git a/http.c b/http.c
index d79678d01f0..25b5fc3b3b7 100644
--- a/http.c
+++ b/http.c
@@ -21,6 +21,7 @@
 #include "string-list.h"
 #include "object-file.h"
 #include "odb.h"
+#include "odb/source-loose.h"
 #include "tempfile.h"
 #include "date.h"
 #include "trace2.h"
diff --git a/loose.c b/loose.c
index d6a2bb5ba3c..64cd6f1c3c1 100644
--- a/loose.c
+++ b/loose.c
@@ -4,6 +4,7 @@
 #include "object-file.h"
 #include "odb.h"
 #include "odb/source-files.h"
+#include "odb/source-loose.h"
 #include "hex.h"
 #include "repository.h"
 #include "wrapper.h"
diff --git a/object-file.c b/object-file.c
index d3ddcbf309c..917a65d5b33 100644
--- a/object-file.c
+++ b/object-file.c
@@ -20,6 +20,7 @@
 #include "object-file-convert.h"
 #include "object-file.h"
 #include "odb.h"
+#include "odb/source.h"
 #include "odb/streaming.h"
 #include "odb/transaction.h"
 #include "pack.h"
diff --git a/object-file.h b/object-file.h
index 83d3f38a540..746356ceb51 100644
--- a/object-file.h
+++ b/object-file.h
@@ -4,7 +4,7 @@
 #include "git-zlib.h"
 #include "object.h"
 #include "odb.h"
-#include "odb/source-loose.h"
+#include "odb/streaming.h"
 #include "odb/transaction.h"
 
 /* The maximum size for an object header. */
diff --git a/setup.c b/setup.c
index 61bcdafe82a..bc94463a239 100644
--- a/setup.c
+++ b/setup.c
@@ -9,6 +9,7 @@
 #include "hex.h"
 #include "object-file.h"
 #include "object-name.h"
+#include "odb/source.h"
 #include "refs.h"
 #include "repository.h"
 #include "config.h"

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 5/6] object-file: stop including "odb/source-loose.h"

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> We have now moved all functionality that relate to the loose backend
> from "object-file.c" into the backend itself. Wrap up these refactorings
> by not including "odb/source-loose.h" anymore and instead include the
> file explicitly in all compilation units that require access to it.
> 
> Like this we can trivially identify exactly those files that still
> depend on details of the loose backend itself and that thus require
> further work to become truly backend-agnostic.
> 

A sensible thing to do.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   builtin/pack-objects.c | 1 +
>   http-walker.c          | 1 +
>   http.c                 | 1 +
>   loose.c                | 1 +
>   object-file.c          | 1 +
>   object-file.h          | 2 +-
>   setup.c                | 1 +
>   7 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index ca3a891dfbc..e53c69af31f 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -34,6 +34,7 @@
>   #include "object-file.h"

I think we can lose "object-file.h" in this one. The code seems to 
compile fine without it.

>   #include "object-file-convert.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   #include "odb/streaming.h"
>   #include "replace-object.h"
>   #include "dir.h"
> diff --git a/http-walker.c b/http-walker.c
> index 4f50dd9b8d0..13730a27847 100644
> --- a/http-walker.c
> +++ b/http-walker.c
> @@ -11,6 +11,7 @@
>   #include "packfile.h"
>   #include "object-file.h"
>   #include "odb.h"
> +#include "odb/source-loose.h"
>   

Ah ha. Looks like I was a bit premature. The http-walker.c change I 
proposed in 2/6 actually belongs here.

>   struct alt_base {
>   	char *base;
> diff --git a/loose.c b/loose.c
> index d6a2bb5ba3c..64cd6f1c3c1 100644
> --- a/loose.c
> +++ b/loose.c
> @@ -4,6 +4,7 @@
>   #include "object-file.h"

Ditto here about losing the object-file.h

>   #include "odb.h"
>   #include "odb/source-files.h"
> +#include "odb/source-loose.h"
>   #include "hex.h"
>   #include "repository.h"
>   #include "wrapper.h"
> diff --git a/object-file.h b/object-file.h
> index 83d3f38a540..746356ceb51 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -4,7 +4,7 @@
>   #include "git-zlib.h"
>   #include "object.h"
>   #include "odb.h"
> -#include "odb/source-loose.h"
> +#include "odb/streaming.h"

Do we really need odb/streaming.h? Code seems to compile and tests are 
happy even without it?

>   #include "odb/transaction.h"
>   
>   /* The maximum size for an object header. */


-- 
Sivaraam

Re: [PATCH 5/6] object-file: stop including "odb/source-loose.h"

[Patrick Steinhardt]

On Tue, Nov 10, 2026 at 09:43:09PM +0530, Kaartic Sivaraam wrote:
> On 11/2/26 16:50, Patrick Steinhardt wrote:
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index ca3a891dfbc..e53c69af31f 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -34,6 +34,7 @@
> >   #include "object-file.h"
> 
> I think we can lose "object-file.h" in this one. The code seems to compile
> fine without it.

Only after step 6 though, because we implicitly depend on "git-zlib.h"
:)

> > diff --git a/http-walker.c b/http-walker.c
> > index 4f50dd9b8d0..13730a27847 100644
> > --- a/http-walker.c
> > +++ b/http-walker.c
> > @@ -11,6 +11,7 @@
> >   #include "packfile.h"
> >   #include "object-file.h"
> >   #include "odb.h"
> > +#include "odb/source-loose.h"
> 
> Ah ha. Looks like I was a bit premature. The http-walker.c change I proposed
> in 2/6 actually belongs here.

Yup, indeed.

> > diff --git a/object-file.h b/object-file.h
> > index 83d3f38a540..746356ceb51 100644
> > --- a/object-file.h
> > +++ b/object-file.h
> > @@ -4,7 +4,7 @@
> >   #include "git-zlib.h"
> >   #include "object.h"
> >   #include "odb.h"
> > -#include "odb/source-loose.h"
> > +#include "odb/streaming.h"
> 
> Do we really need odb/streaming.h? Code seems to compile and tests are happy
> even without it?

If it does then only by chance, as we do use `struct odb_stream` in this
header.

Patrick

[PATCH 6/6] object-file: stop including "git-zlib.h"

[Patrick Steinhardt]

We no longer reference `git_zstream` in "object-file.h", but still
include its header. Drop the header and add it to all the other users
that implicitly depend on it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 apply.c                | 1 +
 builtin/fast-import.c  | 1 +
 builtin/index-pack.c   | 1 +
 builtin/pack-objects.c | 1 +
 diff.c                 | 1 +
 object-file.c          | 1 +
 object-file.h          | 1 -
 odb/source-loose.c     | 1 +
 pack-check.c           | 1 +
 packfile.c             | 1 +
 10 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index a0b7119c639..c705ce06adb 100644
--- a/apply.c
+++ b/apply.c
@@ -21,6 +21,7 @@
 #include "dir.h"
 #include "environment.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "xdiff-interface.h"
 #include "merge-ll.h"
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 9db3f83eea8..c4d955b61f1 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -5,6 +5,7 @@
 #include "abspath.h"
 #include "environment.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "config.h"
 #include "lockfile.h"
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0264c18c5f8..6d758a04419 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -6,6 +6,7 @@
 #include "delta.h"
 #include "environment.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "pack.h"
 #include "csum-file.h"
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e53c69af31f..f13c49090a1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4,6 +4,7 @@
 #include "builtin.h"
 #include "environment.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "config.h"
 #include "attr.h"
diff --git a/diff.c b/diff.c
index b4ac17f8efc..0309f91f5ab 100644
--- a/diff.c
+++ b/diff.c
@@ -18,6 +18,7 @@
 #include "diff.h"
 #include "diffcore.h"
 #include "delta.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "xdiff-interface.h"
 #include "color.h"
diff --git a/object-file.c b/object-file.c
index 917a65d5b33..cf859520949 100644
--- a/object-file.c
+++ b/object-file.c
@@ -15,6 +15,7 @@
 #include "environment.h"
 #include "fsck.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "loose.h"
 #include "object-file-convert.h"
diff --git a/object-file.h b/object-file.h
index 746356ceb51..70138a358de 100644
--- a/object-file.h
+++ b/object-file.h
@@ -1,7 +1,6 @@
 #ifndef OBJECT_FILE_H
 #define OBJECT_FILE_H
 
-#include "git-zlib.h"
 #include "object.h"
 #include "odb.h"
 #include "odb/streaming.h"
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 5cd679e802c..737d583f033 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -3,6 +3,7 @@
 #include "chdir-notify.h"
 #include "dir.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "loose.h"
 #include "object-file.h"
diff --git a/pack-check.c b/pack-check.c
index 605811b7b09..c6c3d7c9c0c 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -2,6 +2,7 @@
 
 #include "git-compat-util.h"
 #include "environment.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "repository.h"
 #include "pack.h"
diff --git a/packfile.c b/packfile.c
index b32b4cd75da..59e0e0c1c91 100644
--- a/packfile.c
+++ b/packfile.c
@@ -3,6 +3,7 @@
 #include "git-compat-util.h"
 #include "environment.h"
 #include "gettext.h"
+#include "git-zlib.h"
 #include "hex.h"
 #include "list.h"
 #include "pack.h"

-- 
2.56.0.500.g5d06dc02cd.dirty

Re: [PATCH 6/6] object-file: stop including "git-zlib.h"

[Kaartic Sivaraam]

On 11/2/26 16:50, Patrick Steinhardt wrote:
> We no longer reference `git_zstream` in "object-file.h", but still
> include its header. Drop the header and add it to all the other users
> that implicitly depend on it.
> 

This looks good and straight-forward to me. Thanks!

-- 
Sivaraam