<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/git.git/cache.h, branch nd/fsck-progress</title>
<subtitle>github.com: git/git.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/'/>
<entry>
<title>Merge branch 'jk/maint-pack-objects-compete-with-delete'</title>
<updated>2011-10-21T23:04:33+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-21T23:04:33+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=207095063379065f2894c091c8909269a00ed44a'/>
<id>207095063379065f2894c091c8909269a00ed44a</id>
<content type='text'>
* jk/maint-pack-objects-compete-with-delete:
  downgrade "packfile cannot be accessed" errors to warnings
  pack-objects: protect against disappearing packs
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* jk/maint-pack-objects-compete-with-delete:
  downgrade "packfile cannot be accessed" errors to warnings
  pack-objects: protect against disappearing packs
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'ph/transport-with-gitfile'</title>
<updated>2011-10-21T23:04:32+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-21T23:04:32+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=afd6284a7fdbb479b5c99a87f64d7496f6fe8a27'/>
<id>afd6284a7fdbb479b5c99a87f64d7496f6fe8a27</id>
<content type='text'>
* ph/transport-with-gitfile:
  Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
  Add test showing git-fetch groks gitfiles
  Teach transport about the gitfile mechanism
  Learn to handle gitfiles in enter_repo
  enter_repo: do not modify input
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* ph/transport-with-gitfile:
  Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
  Add test showing git-fetch groks gitfiles
  Teach transport about the gitfile mechanism
  Learn to handle gitfiles in enter_repo
  enter_repo: do not modify input
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'bc/attr-ignore-case'</title>
<updated>2011-10-18T04:37:14+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-18T04:37:13+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=a200dc8e62563165df1105eae0e980a44059c6fb'/>
<id>a200dc8e62563165df1105eae0e980a44059c6fb</id>
<content type='text'>
* bc/attr-ignore-case:
  attr.c: respect core.ignorecase when matching attribute patterns
  attr: read core.attributesfile from git_default_core_config
  builtin/mv.c: plug miniscule memory leak
  cleanup: use internal memory allocation wrapper functions everywhere
  attr.c: avoid inappropriate access to strbuf "buf" member

Conflicts:
	transport-helper.c
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* bc/attr-ignore-case:
  attr.c: respect core.ignorecase when matching attribute patterns
  attr: read core.attributesfile from git_default_core_config
  builtin/mv.c: plug miniscule memory leak
  cleanup: use internal memory allocation wrapper functions everywhere
  attr.c: avoid inappropriate access to strbuf "buf" member

Conflicts:
	transport-helper.c
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'jk/name-hash-dirent'</title>
<updated>2011-10-18T04:37:11+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-18T04:37:11+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=6f55f0281558f56a0141f3058f245d0877e80efb'/>
<id>6f55f0281558f56a0141f3058f245d0877e80efb</id>
<content type='text'>
* jk/name-hash-dirent:
  fix phantom untracked files when core.ignorecase is set
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* jk/name-hash-dirent:
  fix phantom untracked files when core.ignorecase is set
</pre>
</div>
</content>
</entry>
<entry>
<title>pack-objects: protect against disappearing packs</title>
<updated>2011-10-14T18:42:37+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2011-10-14T18:03:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=4c08018204f95c492c0ad7b033b8f57ccc2e43f5'/>
<id>4c08018204f95c492c0ad7b033b8f57ccc2e43f5</id>
<content type='text'>
It's possible that while pack-objects is running, a
simultaneously running prune process might delete a pack
that we are interested in. Because we load the pack indices
early on, we know that the pack contains our item, but by
the time we try to open and map it, it is gone.

Since c715f78, we already protect against this in the normal
object access code path, but pack-objects accesses the packs
at a lower level.  In the normal access path, we call
find_pack_entry, which will call find_pack_entry_one on each
pack index, which does the actual lookup. If it gets a hit,
we will actually open and verify the validity of the
matching packfile (using c715f78's is_pack_valid). If we
can't open it, we'll issue a warning and pretend that we
didn't find it, causing us to go on to the next pack (or on
to loose objects).

Furthermore, we will cache the descriptor to the opened
packfile. Which means that later, when we actually try to
access the object, we are likely to still have that packfile
opened, and won't care if it has been unlinked from the
filesystem.

Notice the "likely" above. If there is another pack access
in the interim, and we run out of descriptors, we could
close the pack. And then a later attempt to access the
closed pack could fail (we'll try to re-open it, of course,
but it may have been deleted). In practice, this doesn't
happen because we tend to look up items and then access them
immediately.

Pack-objects does not follow this code path. Instead, it
accesses the packs at a much lower level, using
find_pack_entry_one directly. This means we skip the
is_pack_valid check, and may end up with the name of a
packfile, but no open descriptor.

We can add the same is_pack_valid check here. Unfortunately,
the access patterns of pack-objects are not quite as nice
for keeping lookup and object access together. We look up
each object as we find out about it, and the only later when
writing the packfile do we necessarily access it. Which
means that the opened packfile may be closed in the interim.

In practice, however, adding this check still has value, for
three reasons.

  1. If you have a reasonable number of packs and/or a
     reasonable file descriptor limit, you can keep all of
     your packs open simultaneously. If this is the case,
     then the race is impossible to trigger.

  2. Even if you can't keep all packs open at once, you
     may end up keeping the deleted one open (i.e., you may
     get lucky).

  3. The race window is shortened. You may notice early that
     the pack is gone, and not try to access it. Triggering
     the problem without this check means deleting the pack
     any time after we read the list of index files, but
     before we access the looked-up objects.  Triggering it
     with this check means deleting the pack means deleting
     the pack after we do a lookup (and successfully access
     the packfile), but before we access the object. Which
     is a smaller window.

Acked-by: Nicolas Pitre &lt;nico@fluxnic.net&gt;
Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
It's possible that while pack-objects is running, a
simultaneously running prune process might delete a pack
that we are interested in. Because we load the pack indices
early on, we know that the pack contains our item, but by
the time we try to open and map it, it is gone.

Since c715f78, we already protect against this in the normal
object access code path, but pack-objects accesses the packs
at a lower level.  In the normal access path, we call
find_pack_entry, which will call find_pack_entry_one on each
pack index, which does the actual lookup. If it gets a hit,
we will actually open and verify the validity of the
matching packfile (using c715f78's is_pack_valid). If we
can't open it, we'll issue a warning and pretend that we
didn't find it, causing us to go on to the next pack (or on
to loose objects).

Furthermore, we will cache the descriptor to the opened
packfile. Which means that later, when we actually try to
access the object, we are likely to still have that packfile
opened, and won't care if it has been unlinked from the
filesystem.

Notice the "likely" above. If there is another pack access
in the interim, and we run out of descriptors, we could
close the pack. And then a later attempt to access the
closed pack could fail (we'll try to re-open it, of course,
but it may have been deleted). In practice, this doesn't
happen because we tend to look up items and then access them
immediately.

Pack-objects does not follow this code path. Instead, it
accesses the packs at a much lower level, using
find_pack_entry_one directly. This means we skip the
is_pack_valid check, and may end up with the name of a
packfile, but no open descriptor.

We can add the same is_pack_valid check here. Unfortunately,
the access patterns of pack-objects are not quite as nice
for keeping lookup and object access together. We look up
each object as we find out about it, and the only later when
writing the packfile do we necessarily access it. Which
means that the opened packfile may be closed in the interim.

In practice, however, adding this check still has value, for
three reasons.

  1. If you have a reasonable number of packs and/or a
     reasonable file descriptor limit, you can keep all of
     your packs open simultaneously. If this is the case,
     then the race is impossible to trigger.

  2. Even if you can't keep all packs open at once, you
     may end up keeping the deleted one open (i.e., you may
     get lucky).

  3. The race window is shortened. You may notice early that
     the pack is gone, and not try to access it. Triggering
     the problem without this check means deleting the pack
     any time after we read the list of index files, but
     before we access the looked-up objects.  Triggering it
     with this check means deleting the pack means deleting
     the pack after we do a lookup (and successfully access
     the packfile), but before we access the object. Which
     is a smaller window.

Acked-by: Nicolas Pitre &lt;nico@fluxnic.net&gt;
Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'mh/check-ref-format-3'</title>
<updated>2011-10-10T22:56:18+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-10T22:56:18+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=9bd500048d467791902b1a5e8c22165325952fde'/>
<id>9bd500048d467791902b1a5e8c22165325952fde</id>
<content type='text'>
* mh/check-ref-format-3: (23 commits)
  add_ref(): verify that the refname is formatted correctly
  resolve_ref(): expand documentation
  resolve_ref(): also treat a too-long SHA1 as invalid
  resolve_ref(): emit warnings for improperly-formatted references
  resolve_ref(): verify that the input refname has the right format
  remote: avoid passing NULL to read_ref()
  remote: use xstrdup() instead of strdup()
  resolve_ref(): do not follow incorrectly-formatted symbolic refs
  resolve_ref(): extract a function get_packed_ref()
  resolve_ref(): turn buffer into a proper string as soon as possible
  resolve_ref(): only follow a symlink that contains a valid, normalized refname
  resolve_ref(): use prefixcmp()
  resolve_ref(): explicitly fail if a symlink is not readable
  Change check_refname_format() to reject unnormalized refnames
  Inline function refname_format_print()
  Make collapse_slashes() allocate memory for its result
  Do not allow ".lock" at the end of any refname component
  Refactor check_refname_format()
  Change check_ref_format() to take a flags argument
  Change bad_ref_char() to return a boolean value
  ...
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* mh/check-ref-format-3: (23 commits)
  add_ref(): verify that the refname is formatted correctly
  resolve_ref(): expand documentation
  resolve_ref(): also treat a too-long SHA1 as invalid
  resolve_ref(): emit warnings for improperly-formatted references
  resolve_ref(): verify that the input refname has the right format
  remote: avoid passing NULL to read_ref()
  remote: use xstrdup() instead of strdup()
  resolve_ref(): do not follow incorrectly-formatted symbolic refs
  resolve_ref(): extract a function get_packed_ref()
  resolve_ref(): turn buffer into a proper string as soon as possible
  resolve_ref(): only follow a symlink that contains a valid, normalized refname
  resolve_ref(): use prefixcmp()
  resolve_ref(): explicitly fail if a symlink is not readable
  Change check_refname_format() to reject unnormalized refnames
  Inline function refname_format_print()
  Make collapse_slashes() allocate memory for its result
  Do not allow ".lock" at the end of any refname component
  Refactor check_refname_format()
  Change check_ref_format() to take a flags argument
  Change bad_ref_char() to return a boolean value
  ...
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'cb/common-prefix-unification'</title>
<updated>2011-10-10T22:56:17+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-10T22:56:17+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=ca3ef81ad78dcbaecd2bf03e22a5bb9d1fa5eff4'/>
<id>ca3ef81ad78dcbaecd2bf03e22a5bb9d1fa5eff4</id>
<content type='text'>
* cb/common-prefix-unification:
  rename pathspec_prefix() to common_prefix() and move to dir.[ch]
  consolidate pathspec_prefix and common_prefix
  remove prefix argument from pathspec_prefix
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* cb/common-prefix-unification:
  rename pathspec_prefix() to common_prefix() and move to dir.[ch]
  consolidate pathspec_prefix and common_prefix
  remove prefix argument from pathspec_prefix
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'fg/submodule-git-file-git-dir'</title>
<updated>2011-10-10T22:56:17+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-10T22:56:16+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=efc5fb6a77780c53c6636326ca82ff87b17e4c5f'/>
<id>efc5fb6a77780c53c6636326ca82ff87b17e4c5f</id>
<content type='text'>
* fg/submodule-git-file-git-dir:
  Move git-dir for submodules
  rev-parse: add option --resolve-git-dir &lt;path&gt;

Conflicts:
	cache.h
	git-submodule.sh
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* fg/submodule-git-file-git-dir:
  Move git-dir for submodules
  rev-parse: add option --resolve-git-dir &lt;path&gt;

Conflicts:
	cache.h
	git-submodule.sh
</pre>
</div>
</content>
</entry>
<entry>
<title>fix phantom untracked files when core.ignorecase is set</title>
<updated>2011-10-08T00:54:04+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2011-10-06T16:06:09+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=2548183badb98d62079beea62f9d2e1f47e99902'/>
<id>2548183badb98d62079beea62f9d2e1f47e99902</id>
<content type='text'>
When core.ignorecase is turned on and there are stale index
entries, "git commit" can sometimes report directories as
untracked, even though they contain tracked files.

You can see an example of this with:

    # make a case-insensitive repo
    git init repo &amp;&amp; cd repo &amp;&amp;
    git config core.ignorecase true &amp;&amp;

    # with some tracked files in a subdir
    mkdir subdir &amp;&amp;
    &gt; subdir/one &amp;&amp;
    &gt; subdir/two &amp;&amp;
    git add . &amp;&amp;
    git commit -m base &amp;&amp;

    # now make the index entries stale
    touch subdir/* &amp;&amp;

    # and then ask commit to update those entries and show
    # us the status template
    git commit -a

which will report "subdir/"  as untracked, even though it
clearly contains two tracked files. What is happening in the
commit program is this:

  1. We load the index, and for each entry, insert it into the index's
     name_hash. In addition, if ignorecase is turned on, we make an
     entry in the name_hash for the directory (e.g., "contrib/"), which
     uses the following code from 5102c61's hash_index_entry_directories:

        hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
        if (!lookup_hash(hash, &amp;istate-&gt;name_hash)) {
                pos = insert_hash(hash, &amp;istate-&gt;name_hash);
		if (pos) {
			ce-&gt;next = *pos;
			*pos = ce;
		}
        }

     Note that we only add the directory entry if there is not already an
     entry.

  2. We run add_files_to_cache, which gets updated information for each
     cache entry. It helpfully inserts this information into the cache,
     which calls replace_index_entry. This in turn calls
     remove_name_hash() on the old entry, and add_name_hash() on the new
     one. But remove_name_hash doesn't actually remove from the hash, it
     only marks it as "no longer interesting" (from cache.h):

      /*
       * We don't actually *remove* it, we can just mark it invalid so that
       * we won't find it in lookups.
       *
       * Not only would we have to search the lists (simple enough), but
       * we'd also have to rehash other hash buckets in case this makes the
       * hash bucket empty (common). So it's much better to just mark
       * it.
       */
      static inline void remove_name_hash(struct cache_entry *ce)
      {
              ce-&gt;ce_flags |= CE_UNHASHED;
      }

     This is OK in the specific-file case, since the entries in the hash
     form a linked list, and we can just skip the "not here anymore"
     entries during lookup.

     But for the directory hash entry, we will _not_ write a new entry,
     because there is already one there: the old one that is actually no
     longer interesting!

  3. While traversing the directories, we end up in the
     directory_exists_in_index_icase function to see if a directory is
     interesting. This in turn checks index_name_exists, which will
     look up the directory in the index's name_hash. We see the old,
     deleted record, and assume there is nothing interesting. The
     directory gets marked as untracked, even though there are index
     entries in it.

The problem is in the code I showed above:

        hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
        if (!lookup_hash(hash, &amp;istate-&gt;name_hash)) {
                pos = insert_hash(hash, &amp;istate-&gt;name_hash);
		if (pos) {
			ce-&gt;next = *pos;
			*pos = ce;
		}
        }

Having a single cache entry that represents the directory is
not enough; that entry may go away if the index is changed.
It may be tempting to say that the problem is in our removal
method; if we removed the entry entirely instead of simply
marking it as "not here anymore", then we would know we need
to insert a new entry. But that only covers this particular
case of remove-replace. In the more general case, consider
something like this:

  1. We add "foo/bar" and "foo/baz" to the index. Each gets
     their own entry in name_hash, plus we make a "foo/"
     entry that points to "foo/bar".

  2. We remove the "foo/bar" entry from the index, and from
     the name_hash.

  3. We ask if "foo/" exists, and see no entry, even though
     "foo/baz" exists.

So we need that directory entry to have the list of _all_
cache entries that indicate that the directory is tracked.
So that implies making a linked list as we do for other
entries, like:

  hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
  pos = insert_hash(hash, &amp;istate-&gt;name_hash);
  if (pos) {
	  ce-&gt;next = *pos;
	  *pos = ce;
  }

But that's not right either. In fact, it shows a second bug
in the current code, which is that the "ce-&gt;next" pointer is
supposed to be linking entries for a specific filename
entry, but here we are overwriting it for the directory
entry. So the same cache entry ends up in two linked lists,
but they share the same "next" pointer.

As it turns out, this second bug can't be triggered in the
current code. The "if (pos)" conditional is totally dead
code; pos will only be non-NULL if there was an existing
hash entry, and we already checked that there wasn't one
through our call to lookup_hash.

But fixing the first bug means taking out that call to
lookup_hash, which is going to activate the buggy dead code,
and we'll end up splicing the two linked lists together.

So we need to have a separate next pointer for the list in
the directory bucket, and we need to traverse that list in
index_name_exists when we are looking up a directory.

This bloats "struct cache_entry" by a few bytes. Which is
annoying, because it's only necessary when core.ignorecase
is enabled. There's not an easy way around it, short of
separating out the "next" pointers from cache_entry entirely
(i.e., having a separate "cache_entry_list" struct that gets
stored in the name_hash). In practice, it probably doesn't
matter; we have thousands of cache entries, compared to the
millions of objects (where adding 4 bytes to the struct
actually does impact performance).

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When core.ignorecase is turned on and there are stale index
entries, "git commit" can sometimes report directories as
untracked, even though they contain tracked files.

You can see an example of this with:

    # make a case-insensitive repo
    git init repo &amp;&amp; cd repo &amp;&amp;
    git config core.ignorecase true &amp;&amp;

    # with some tracked files in a subdir
    mkdir subdir &amp;&amp;
    &gt; subdir/one &amp;&amp;
    &gt; subdir/two &amp;&amp;
    git add . &amp;&amp;
    git commit -m base &amp;&amp;

    # now make the index entries stale
    touch subdir/* &amp;&amp;

    # and then ask commit to update those entries and show
    # us the status template
    git commit -a

which will report "subdir/"  as untracked, even though it
clearly contains two tracked files. What is happening in the
commit program is this:

  1. We load the index, and for each entry, insert it into the index's
     name_hash. In addition, if ignorecase is turned on, we make an
     entry in the name_hash for the directory (e.g., "contrib/"), which
     uses the following code from 5102c61's hash_index_entry_directories:

        hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
        if (!lookup_hash(hash, &amp;istate-&gt;name_hash)) {
                pos = insert_hash(hash, &amp;istate-&gt;name_hash);
		if (pos) {
			ce-&gt;next = *pos;
			*pos = ce;
		}
        }

     Note that we only add the directory entry if there is not already an
     entry.

  2. We run add_files_to_cache, which gets updated information for each
     cache entry. It helpfully inserts this information into the cache,
     which calls replace_index_entry. This in turn calls
     remove_name_hash() on the old entry, and add_name_hash() on the new
     one. But remove_name_hash doesn't actually remove from the hash, it
     only marks it as "no longer interesting" (from cache.h):

      /*
       * We don't actually *remove* it, we can just mark it invalid so that
       * we won't find it in lookups.
       *
       * Not only would we have to search the lists (simple enough), but
       * we'd also have to rehash other hash buckets in case this makes the
       * hash bucket empty (common). So it's much better to just mark
       * it.
       */
      static inline void remove_name_hash(struct cache_entry *ce)
      {
              ce-&gt;ce_flags |= CE_UNHASHED;
      }

     This is OK in the specific-file case, since the entries in the hash
     form a linked list, and we can just skip the "not here anymore"
     entries during lookup.

     But for the directory hash entry, we will _not_ write a new entry,
     because there is already one there: the old one that is actually no
     longer interesting!

  3. While traversing the directories, we end up in the
     directory_exists_in_index_icase function to see if a directory is
     interesting. This in turn checks index_name_exists, which will
     look up the directory in the index's name_hash. We see the old,
     deleted record, and assume there is nothing interesting. The
     directory gets marked as untracked, even though there are index
     entries in it.

The problem is in the code I showed above:

        hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
        if (!lookup_hash(hash, &amp;istate-&gt;name_hash)) {
                pos = insert_hash(hash, &amp;istate-&gt;name_hash);
		if (pos) {
			ce-&gt;next = *pos;
			*pos = ce;
		}
        }

Having a single cache entry that represents the directory is
not enough; that entry may go away if the index is changed.
It may be tempting to say that the problem is in our removal
method; if we removed the entry entirely instead of simply
marking it as "not here anymore", then we would know we need
to insert a new entry. But that only covers this particular
case of remove-replace. In the more general case, consider
something like this:

  1. We add "foo/bar" and "foo/baz" to the index. Each gets
     their own entry in name_hash, plus we make a "foo/"
     entry that points to "foo/bar".

  2. We remove the "foo/bar" entry from the index, and from
     the name_hash.

  3. We ask if "foo/" exists, and see no entry, even though
     "foo/baz" exists.

So we need that directory entry to have the list of _all_
cache entries that indicate that the directory is tracked.
So that implies making a linked list as we do for other
entries, like:

  hash = hash_name(ce-&gt;name, ptr - ce-&gt;name);
  pos = insert_hash(hash, &amp;istate-&gt;name_hash);
  if (pos) {
	  ce-&gt;next = *pos;
	  *pos = ce;
  }

But that's not right either. In fact, it shows a second bug
in the current code, which is that the "ce-&gt;next" pointer is
supposed to be linking entries for a specific filename
entry, but here we are overwriting it for the directory
entry. So the same cache entry ends up in two linked lists,
but they share the same "next" pointer.

As it turns out, this second bug can't be triggered in the
current code. The "if (pos)" conditional is totally dead
code; pos will only be non-NULL if there was an existing
hash entry, and we already checked that there wasn't one
through our call to lookup_hash.

But fixing the first bug means taking out that call to
lookup_hash, which is going to activate the buggy dead code,
and we'll end up splicing the two linked lists together.

So we need to have a separate next pointer for the list in
the directory bucket, and we need to traverse that list in
index_name_exists when we are looking up a directory.

This bloats "struct cache_entry" by a few bytes. Which is
annoying, because it's only necessary when core.ignorecase
is enabled. There's not an easy way around it, short of
separating out the "next" pointers from cache_entry entirely
(i.e., having a separate "cache_entry_list" struct that gets
stored in the name_hash). In practice, it probably doesn't
matter; we have thousands of cache entries, compared to the
millions of objects (where adding 4 bytes to the struct
actually does impact performance).

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>attr: read core.attributesfile from git_default_core_config</title>
<updated>2011-10-06T20:54:32+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2011-10-06T18:22:24+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=64589a03a8ffb3eb4fb2ff8f416ff638a9aaa439'/>
<id>64589a03a8ffb3eb4fb2ff8f416ff638a9aaa439</id>
<content type='text'>
This code calls git_config from a helper function to parse the config entry
it is interested in.  Calling git_config in this way may cause a problem if
the helper function can be called after a previous call to git_config by
another function since the second call to git_config may reset some
variable to the value in the config file which was previously overridden.

The above is not a problem in this case since the function passed to
git_config only parses one config entry and the variable it sets is not
assigned outside of the parsing function.  But a programmer who desires
all of the standard config options to be parsed may be tempted to modify
git_attr_config() so that it falls back to git_default_config() and then it
_would_ be vulnerable to the above described behavior.

So, move the call to git_config up into the top-level cmd_* function and
move the responsibility for parsing core.attributesfile into the main
config file parser.

Which is only the logical thing to do ;-)

Signed-off-by: Brandon Casey &lt;drafnel@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This code calls git_config from a helper function to parse the config entry
it is interested in.  Calling git_config in this way may cause a problem if
the helper function can be called after a previous call to git_config by
another function since the second call to git_config may reset some
variable to the value in the config file which was previously overridden.

The above is not a problem in this case since the function passed to
git_config only parses one config entry and the variable it sets is not
assigned outside of the parsing function.  But a programmer who desires
all of the standard config options to be parsed may be tempted to modify
git_attr_config() so that it falls back to git_default_config() and then it
_would_ be vulnerable to the above described behavior.

So, move the call to git_config up into the top-level cmd_* function and
move the responsibility for parsing core.attributesfile into the main
config file parser.

Which is only the logical thing to do ;-)

Signed-off-by: Brandon Casey &lt;drafnel@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
