Linus,
sorry for bringing up an issue that is already 8 hours old.
LT> I don't think that's a good interface. It changes the sha1 passed into it:
LT> that may actually be nice, since you may want to know what it changed to,
LT> but I think you'd want to have that as an (optional) separate
LT> "sha1_result" parameter.
Point taken about "_changing_ _is_ _bad_" part. It was a mistake.
LT> Also, the "type" or "size" things make no sense to have as a parameter
LT> at all.
Well, the semantics is "I want to read the raw data of a tree
and I do not know nor care if this sha1 I got from my user is
for a commit or a tree." So type does not matter (if it returns
a non NULL we know it is a tree), but the size matters.
And that semantics is not so hacky thing specific to diff-cache.
Rather, it applies in general if you structure the way those
recursive walkers do things. The recursive walkers in ls-tree,
diff-cache, and diff-tree all expect the caller to supply the
buffer read by sha1_read_buffer, and when it calls itself it
does the same (read-tree's recursing convention is an oddball
that needs to be addressed, though).
When the recursion is structured this way, the only thing you
need to do to allow commit ID from the user when tree ID is
needed, without breaking the error checking done by the part
that recurses down (i.e. we must error on a commit object ID
when we are expecting a tree object ID stored in objects we read
from the tree downwards), is to change the top-level caller to
use "I want tree with this tree/commit ID" instead of "I want a
buffer with this ID and I'll make sure it is a tree myself".
Instead, you make the recursor "Give me a buffer and its type,
I'll barf if it is does not say a tree." When the recursor
calls itself, it reads with read_sha1_file and feeds the result
to itself and have the called do the checking.
The commit_to_tree() thing you introduced in diff-tree.c is
simple to use. IMHO it is however conceptually a wrong thing to
use in these contexts. When the user supplies a tree ID, you
first read that object only to see if it is not a commit and
throw it away, then immediately read it again for your real
processing. In these particular cases of four tree- related
files, "I want tree with this tree/commit ID" semantics is a
_far_ _better_ match for the problem.
Having said that, here is a reworked version. This first one
introduces read_tree_with_tree_or_commit_sha1() function.
<end-of-cover-letter>
This patch implements read_tree_with_tree_or_commit_sha1(),
which can be used when you are interested in reading an unpacked
raw tree data but you do not know nor care if the SHA1 you
obtained your user is a tree ID or a commit ID. Before this
function's introduction, you would have called read_sha1_file(),
examined its type, parsed it to call read_sha1_file() again if
it is a commit, and verified that the resulting object is a
tree. Instead, this function does that for you. It returns
NULL if the given SHA1 is not either a tree or a commit.
Signed-off-by: Junio C Hamano <[EMAIL PROTECTED]>
---
cache.h | 4 ++++
sha1_file.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
cache.h: eab355da5d2f6595053f28f0cca61181ac314ee9
--- a/cache.h
+++ b/cache.h
@@ -124,4 +124,8 @@ extern int error(const char *err, ...);
extern int cache_name_compare(const char *name1, int len1, const char *name2,
int len2);
+extern void *read_tree_with_tree_or_commit_sha1(const unsigned char *sha1,
+ unsigned long *size,
+ unsigned char *tree_sha1_ret);
+
#endif /* CACHE_H */
sha1_file.c: eee3598bb75e2199045b823f007e7933c0fb9cfe
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -166,6 +166,46 @@ void * read_sha1_file(const unsigned cha
return NULL;
}
+void *read_tree_with_tree_or_commit_sha1(const unsigned char *sha1,
+ unsigned long *size,
+ unsigned char *tree_sha1_return)
+{
+ char type[20];
+ void *buffer;
+ unsigned long isize;
+ int was_commit = 0;
+ char tree_sha1[20];
+
+ buffer = read_sha1_file(sha1, type, &isize);
+
+ /*
+ * We might have read a commit instead of a tree, in which case
+ * we parse out the tree_sha1 and attempt to read from there.
+ * (buffer + 5) is because the tree sha1 is always at offset 5
+ * in a commit record ("tree ").
+ */
+ if (buffer &&
+ !strcmp(type, "commit") &&
+ !get_sha1_hex(buffer + 5, tree_sha1)) {
+ free(buffer);
+ buffer = read_sha1_file(tree_sha1, type, &isize);
+ was_commit = 1;
+ }
+
+ /*
+ * Now do we have something and if so is it a tree?
+ */
+ if (!buffer || strcmp(type, "tree")) {
+ free(buffer);
+ return;
+ }
+
+ *size = isize;
+ if (tree_sha1_return)
+ memcpy(tree_sha1_return, was_commit ? tree_sha1 : sha1, 20);
+ return buffer;
+}
+
int write_sha1_file(char *buf, unsigned len, unsigned char *returnsha1)
{
int size;
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
