Re: [PATCH v3 10/11] read-cache.c: fix memory leaks caused by removed cache entries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Karsten Blees <karsten.blees@xxxxxxxxx> writes:

> When cache_entry structs are removed from index_state.cache, they are not
> properly freed. Freeing those entries wasn't possible before because we
> couldn't remove them from index_state.name_hash.
>
> Now that we _do_ remove the entries from name_hash, we can also free them.
> Add free(cache_entry) to all call sites of name-hash.c::remove_name_hash in
> read-cache.c, as name-hash.c isn't concerned with cache_entry allocation.
>
> cmd_rm and unmerge_index_entry_at use cache_entry.name after removing the
> entry. Copy the name so that we don't access memory that has been freed.

Is this the version that is currently in pu?  There's a valgrind test
failure in current pu that bisects to 36850edb, which would seem to be
from this email but doesn't have the right author date.  Worse, I cannot
apply this on top of 36850edb^ because I don't have the 'index' blobs
for this patch.  Confusing.

In any case 36850edb currently breaks several valgrind tests.  You can
valgrind only t6022.16 like so (that one test is sufficient to track it
down and it's much faster that way):

  cd t  
  ./t6022-merge-rename.sh --valgrind-only=16

The valgrind error in t6022.16 looks like this:

  ==4959== Invalid read of size 1
  ==4959==    at 0x5682A38: vfprintf (vfprintf.c:1629)
  ==4959==    by 0x56AC564: vsnprintf (vsnprintf.c:119)
  ==4959==    by 0x542005: vreportf (usage.c:12)
  ==4959==    by 0x54216C: error_builtin (usage.c:42)
  ==4959==    by 0x54261B: error (usage.c:147)
  ==4959==    by 0x4FC681: read_index_unmerged (read-cache.c:1900)
  ==4959==    by 0x475CF1: reset_index (reset.c:68)
  ==4959==    by 0x476A72: cmd_reset (reset.c:346)
  ==4959==    by 0x405999: run_builtin (git.c:314)
  ==4959==    by 0x405B2C: handle_internal_command (git.c:477)
  ==4959==    by 0x405C46: run_argv (git.c:523)
  ==4959==    by 0x405DE2: main (git.c:606)
  ==4959==  Address 0x5bedb54 is 84 bytes inside a block of size 104 free'd
  ==4959==    at 0x4C2ACDA: free (vg_replace_malloc.c:468)
  ==4959==    by 0x4F9360: remove_index_entry_at (read-cache.c:482)
  ==4959==    by 0x4FA469: add_index_entry_with_check (read-cache.c:964)
  ==4959==    by 0x4FA5A4: add_index_entry (read-cache.c:993)
  ==4959==    by 0x4FC663: read_index_unmerged (read-cache.c:1899)
  ==4959==    by 0x475CF1: reset_index (reset.c:68)
  ==4959==    by 0x476A72: cmd_reset (reset.c:346)
  ==4959==    by 0x405999: run_builtin (git.c:314)
  ==4959==    by 0x405B2C: handle_internal_command (git.c:477)
  ==4959==    by 0x405C46: run_argv (git.c:523)
  ==4959==    by 0x405DE2: main (git.c:606)

If you need any more information/help, just ask :-)

-- 
Thomas Rast
tr@xxxxxxxxxxxxx
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]