On Mon, Dec 09, 2013 at 03:37:50PM -0800, Junio C Hamano wrote:
> > +void submodule_config_cache_free(struct submodule_config_cache *cache)
> > +{
> > + /* NOTE: its important to iterate over the name hash here
> > + * since paths might have multiple entries */
>
> Style (multi-line comments).
Will fix.
> This is interesting. I wonder what the practical consequence is to
> have a single submodule bound to the top-level tree more than once.
> Updating from one of the working tree will make the other working
> tree out of sync because the ultimate location of the submodule
> directory pointed at by the two .git gitdirs can only have a single
> HEAD, be it detached or on a branch, and a single index.
To clarify, when writing this comment I was not thinking about the same
submodule with multiple paths in the same tree but rather with the same
name under different paths in different commits.
> Not that the decision to enforce that names are unique in the
> top-level .gitmodules, and follow that decision in this part of the
> code to be defensive (not rely on the "one submodule can be bound
> only once to a top-level tree"), but shouldn't such a configuration
> to have a single submodule bound to more than one place in the
> top-level tree be forbidden?
Yes IMO, that should be forbidden currently. I do not think we actually
prevent the user from doing so but it can not happen by accident since
we derive the initial name from the local path. Maybe we should be more
strict about that and put more guards in place to avoid such
configurations from entering the database.
> > + for_each_hash(&cache->for_name, free_one_submodule_config, NULL);
> > + free_hash(&cache->for_path);
> > + free_hash(&cache->for_name);
> > +}
> > +
> > +static unsigned int hash_sha1_string(const unsigned char *sha1, const char *string)
> > +{
> > + int c;
> > + unsigned int hash, string_hash = 5381;
> > + memcpy(&hash, sha1, sizeof(hash));
> > +
> > + /* djb2 hash */
> > + while ((c = *string++))
> > + string_hash = ((string_hash << 5) + hash) + c; /* hash * 33 + c */
>
> Hmm, the comment and the code does not seem to match in math here...
Yeah sorry that was a leftover from the code I started with. In the
beginning it was a pure string hash. Will remove both comments (since
its also not a pure djb2 hash anymore).
Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html