Re: [PATCH v2 01/10] Define a structure for object IDs.

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

I'd advise against it.

As I wrote in $gmane/265337 in response to Michael:

    > 4. We continue to support working with SHA-1s declared to be (unsigned
    > char *) in some performance-critical code, even as we migrate most other
    > code to using SHA-1s embedded within a (struct object_id). This will
    > cost some duplication of code. To accept this approach, we would need an
    > idea of *how much* code duplication would be needed. E.g., how many
    > functions will need both (unsigned char *) versions and (struct
    > object_id *) versions?

    ...

    I do not know what kind of code duplication you are worried about,
    though.  If a callee needs "unsigned char *", the caller that has a
    "struct object_id *o" should pass o->hash to the callee.

And that would break the abstraction effort if you start calling the field with a name that is specific to the underlying hash function. The caller has to change o->sha1 to o->sha256 instead of keeping that as o->oid and letting the callee handle the implementation details when calling

        if (!hashcmp(o1->oid, o2->oid))
                ; /* they are the same */
        else
                ; /* they are different */

The only folks that need to _know_ what hash function is used or how long the field is are the ones that have raw bytes of the hash obtained from files (e.g. from the index) and they would do something like this to implement a function that checks the file records an object name that is expected by the caller:

        void check_oid(int fd, struct object_id *expected)
        {
                unsigned char object_name[GIT_HASH_RAWSZ];

                ...
                read(fd, object_name, GIT_HASH_RAWSZ);
                if (hashcmp(object_name, expected->oid))
                        die("object name mismatch???");
        }

or when they do know they are using a specific function, do:

        void compute_object_name(struct object_id *id,
                                unsignd char*data,
                                size_t len)
        {
                git_SHA_CTX c;
                unsigned char *sha1 = &(id->oid);

                /* if we are paranoid... */
                assert(sizeof(id->oid) >= 20);

                ...
                git_SHA1_Init(&c);
                git_SHA1_Update(&c, data, len);
                ...
                git_SHA1_Final(sha1, &c);
        }

Even the latter would not be helped if the field to store the hash were named id->sha1 very much, I would think.