On Sun, Oct 01, 2023 at 09:40:05PM -0500, Eric W. Biederman wrote: > From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > > Two basic functions are provided: > - convert_object_file Takes an object file it's type and hash algorithm > and converts it into the equivalent object file that would > have been generated with hash algorithm "to". > > For blob objects there is no conversation to be done and it is an > error to use this function on them. > > For commit, tree, and tag objects embedded oids are replaced by the > oids of the objects they refer to with those objects and their > object ids reencoded in with the hash algorithm "to". Signatures > are rearranged so that they remain valid after the object has > been reencoded. > > - repo_oid_to_algop which takes an oid that refers to an object file > and returns the oid of the equivalent object file generated > with the target hash algorithm. > > The pair of files object-file-convert.c and object-file-convert.h are > introduced to hold as much of this logic as possible to keep this > conversion logic cleanly separated from everything else and in the > hopes that someday the code will be clean enough git can support > compiling out support for sha1 and the various conversion functions. > > Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > --- > Makefile | 1 + > object-file-convert.c | 57 +++++++++++++++++++++++++++++++++++++++++++ > object-file-convert.h | 24 ++++++++++++++++++ > 3 files changed, 82 insertions(+) > create mode 100644 object-file-convert.c > create mode 100644 object-file-convert.h > > diff --git a/Makefile b/Makefile > index 577630936535..f7e824f25cda 100644 > --- a/Makefile > +++ b/Makefile > @@ -1073,6 +1073,7 @@ LIB_OBJS += notes-cache.o > LIB_OBJS += notes-merge.o > LIB_OBJS += notes-utils.o > LIB_OBJS += notes.o > +LIB_OBJS += object-file-convert.o > LIB_OBJS += object-file.o > LIB_OBJS += object-name.o > LIB_OBJS += object.o > diff --git a/object-file-convert.c b/object-file-convert.c > new file mode 100644 > index 000000000000..4777aba83636 > --- /dev/null > +++ b/object-file-convert.c > @@ -0,0 +1,57 @@ > +#include "git-compat-util.h" > +#include "gettext.h" > +#include "strbuf.h" > +#include "repository.h" > +#include "hash-ll.h" > +#include "object.h" > +#include "object-file-convert.h" > + > +int repo_oid_to_algop(struct repository *repo, const struct object_id *src, > + const struct git_hash_algo *to, struct object_id *dest) > +{ > + /* > + * If the source algorithm is not set, then we're using the > + * default hash algorithm for that object. > + */ > + const struct git_hash_algo *from = > + src->algo ? &hash_algos[src->algo] : repo->hash_algo; > + > + if (from == to) { > + if (src != dest) > + oidcpy(dest, src); > + return 0; > + } > + return -1; > +} In it's current form, `repo_oid_to_algop()` basically never does anything except for copying over the object ID because we do not handle the case where object hashes are different. I assume this is intended, as we basically only provide stubs in this commit. But still, it would help to document this in-code as well with a comment. > +int convert_object_file(struct strbuf *outbuf, > + const struct git_hash_algo *from, > + const struct git_hash_algo *to, > + const void *buf, size_t len, > + enum object_type type, > + int gentle) > +{ > + int ret; > + > + /* Don't call this function when no conversion is necessary */ > + if ((from == to) || (type == OBJ_BLOB)) > + BUG("Refusing noop object file conversion"); The extra braces around comparisons are unneeded and to the best of my knowledge not customary in our code base. Also, error messages should start with a lower-case letter. > + switch (type) { > + case OBJ_COMMIT: > + case OBJ_TREE: > + case OBJ_TAG: > + default: > + /* Not implemented yet, so fail. */ > + ret = -1; > + break; > + } It's a bit weird that we handle all object types except for blobs separately, and then still have a `default` statement. I would've thought that we should handle the object types specifically and set `ret = -1` for all of them, and then the `default` case would instead call `BUG()` due to an unknown object type. > + if (!ret) > + return 0; > + if (gentle) { > + strbuf_release(outbuf); > + return ret; > + } Do you really intend to call `strbuf_release()` on the caller provided buffer, or should this rather be `strbuf_reset()`? Memory management of such an in/out parameter should typically be handled by the caller, not the callee. > + die(_("Failed to convert object from %s to %s"), > + from->name, to->name); > +} The error message should start with a lower-case letter. > diff --git a/object-file-convert.h b/object-file-convert.h > new file mode 100644 > index 000000000000..a4f802aa8eea > --- /dev/null > +++ b/object-file-convert.h > @@ -0,0 +1,24 @@ > +#ifndef OBJECT_CONVERT_H > +#define OBJECT_CONVERT_H > + > +struct repository; > +struct object_id; > +struct git_hash_algo; > +struct strbuf; > +#include "object.h" > + > +int repo_oid_to_algop(struct repository *repo, const struct object_id *src, > + const struct git_hash_algo *to, struct object_id *dest); > + > +/* > + * Convert an object file from one hash algorithm to another algorithm. > + * Return -1 on failure, 0 on success. > + */ > +int convert_object_file(struct strbuf *outbuf, > + const struct git_hash_algo *from, > + const struct git_hash_algo *to, > + const void *buf, size_t len, > + enum object_type type, > + int gentle); It would be nice to document what `gentle` does. Patrick > +#endif /* OBJECT_CONVERT_H */ > -- > 2.41.0 >
Attachment:
signature.asc
Description: PGP signature