On Thu, Dec 08, 2022 at 12:57:06PM -0800, Jonathan Tan wrote:
> This function can do 3 things:
> 1. Gets an fd given a path
> 2. Simultaneously gets a path and fd given an OID
> 3. Memory maps an fd
>
> Split this function up. Only one caller needs 1, so inline that. As for
> 2, a future patch will also need this functionality and, in addition,
> the calculated path, so extract this into a separate function with an
> out parameter for the path.
This is moving in a good direction. I like the name "map_fd" for the
helper. Being able to give it a useful name like that is a good clue
that it is doing a more focused and understandable job than the generic
map_loose_object_1(). :)
In fact...
> +static void *map_loose_object_1(struct repository *r,
> + const struct object_id *oid,
> + unsigned long *size,
> + const char **path)
> +{
> + const char *p;
> + int fd = open_loose_object(r, oid, &p);
> +
> + if (fd < 0)
> + return NULL;
> + if (path)
> + *path = p;
> + return map_fd(fd, p, size);
> +}
> +
> void *map_loose_object(struct repository *r,
> const struct object_id *oid,
> unsigned long *size)
> {
> - return map_loose_object_1(r, NULL, oid, size);
> + return map_loose_object_1(r, oid, size, NULL);
> }
If you take my suggestion on patch 3, then the only other caller of
map_loose_object_1() goes away, and we can fold it all into one
reasonably-named function:
diff --git a/object-file.c b/object-file.c
index d99d05839f..429e3a746d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1233,28 +1233,18 @@ static void *map_fd(int fd, const char *path, unsigned long *size)
return map;
}
-static void *map_loose_object_1(struct repository *r,
- const struct object_id *oid,
- unsigned long *size,
- const char **path)
+void *map_loose_object(struct repository *r,
+ const struct object_id *oid,
+ unsigned long *size)
{
const char *p;
int fd = open_loose_object(r, oid, &p);
if (fd < 0)
return NULL;
- if (path)
- *path = p;
return map_fd(fd, p, size);
}
-void *map_loose_object(struct repository *r,
- const struct object_id *oid,
- unsigned long *size)
-{
- return map_loose_object_1(r, oid, size, NULL);
-}
-
enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
unsigned char *map,
unsigned long mapsize,
-Peff
