Skip to content
  • Martin Ågren's avatar
    object_array: add and use `object_array_pop()` · 71992039
    Martin Ågren authored and Junio C Hamano's avatar Junio C Hamano committed
    
    
    In a couple of places, we pop objects off an object array `foo` by
    decreasing `foo.nr`. We access `foo.nr` in many places, but most if not
    all other times we do so read-only, e.g., as we iterate over the array.
    But when we change `foo.nr` behind the array's back, it feels a bit
    nasty and looks like it might leak memory.
    
    Leaks happen if the popped element has an allocated `name` or `path`.
    At the moment, that is not the case. Still, 1) the object array might
    gain more fields that want to be freed, 2) a code path where we pop
    might start using names or paths, 3) one of these code paths might be
    copied to somewhere where we do, and 4) using a dedicated function for
    popping is conceptually cleaner.
    
    Introduce and use `object_array_pop()` instead. Release memory in the
    new function. Document that popping an object leaves the associated
    elements in limbo.
    
    The converted places were identified by grepping for "\.nr\>" and
    looking for "--".
    
    Make the new function return NULL on an empty array. This is consistent
    with `pop_commit()` and allows the following:
    
    	while ((o = object_array_pop(&foo)) != NULL) {
    		// do something
    	}
    
    But as noted above, we don't need to go out of our way to avoid reading
    `foo.nr`. This is probably more readable:
    
    	while (foo.nr) {
    		... o = object_array_pop(&foo);
    		// do something
    	}
    
    The name of `object_array_pop()` does not quite align with
    `add_object_array()`. That is unfortunate. On the other hand, it matches
    `object_array_clear()`. Arguably it's `add_...` that is the odd one out,
    since it reads like it's used to "add" an "object array". For that
    reason, side with `object_array_clear()`.
    
    Signed-off-by: default avatarMartin Ågren <martin.agren@gmail.com>
    Reviewed-by: default avatarJeff King <peff@peff.net>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    71992039