Skip to content
  • Michael Haggerty's avatar
    object_array_entry: fix memory handling of the name field · 31faeb20
    Michael Haggerty authored and Junio C Hamano's avatar Junio C Hamano committed
    Previously, the memory management of the object_array_entry::name
    field was inconsistent and undocumented.  object_array_entries are
    ultimately created by a single function, add_object_array_with_mode(),
    which has an argument "const char *name".  This function used to
    simply set the name field to reference the string pointed to by the
    name parameter, and nobody on the object_array side ever freed the
    memory.  Thus, it assumed that the memory for the name field would be
    managed by the caller, and that the lifetime of that string would be
    at least as long as the lifetime of the object_array_entry.  But
    callers were inconsistent:
    
    * Some passed pointers to constant strings or argv entries, which was
      OK.
    
    * Some passed pointers to newly-allocated memory, but didn't arrange
      for the memory ever to be freed.
    
    * Some passed the return value of sha1_to_hex(), which is a pointer to
      a statically-allocated buffer that can be overwritten at any time.
    
    * Some passed pointers to refnames that they received from a
      for_each_ref()-type iteration, but the lifetimes of such refnames is
      not guaranteed by the refs API.
    
    Bring consistency to this mess by changing object_array to make its
    own copy for the object_array_entry::name field and free this memory
    when an object_array_entry is deleted from the array.
    
    Many callers were passing the empty string as the name parameter, so
    as a performance optimization, treat the empty string specially.
    Instead of making a copy, store a pointer to a statically-allocated
    empty string to object_array_entry::name.  When deleting such an
    entry, skip the free().
    
    Change the callers that were already passing copies to
    add_object_array_with_mode() to either skip the copy, or (if the
    memory needed to be allocated anyway) freeing the memory itself.
    
    A part of this commit effectively reverts
    
        70d26c6e
    
     read_revisions_from_stdin: make copies for handle_revision_arg
    
    because the copying introduced by that commit (which is still
    necessary) is now done at a deeper level.
    
    Signed-off-by: default avatarMichael Haggerty <mhagger@alum.mit.edu>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    31faeb20