git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 25/25] refs: document the lifetime of the args passed to each_ref_fn

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 25, 2013, 09:08 UTC
Message-ID
<1369472904-12875-26-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1369472904-12875-1-git-send-email-mhagger@alum.mit.edu>

The lifetime of the memory pointed to by the refname and sha1 arguments to each_ref_fn was never documented, but some callers used to assume that it was essentially permanent. In fact the API does *not* guarantee that these objects live beyond a single callback invocation.

In the current code, the lifetimes are bound together with the lifetimes of the ref_caches. Since these are usually long, the callers usually got away with their sloppiness. But even today, if a ref_cache is invalidated the memory can be freed. And planned changes to reference caching, needed to eliminate race conditions, will probably need to shorten the lifetimes of these objects.

The commits leading up to this have (hopefully) fixed all of the callers of the for_each_ref()-like functions. This commit does the last step: documents what each_ref_fn callbacks can assume about object lifetimes.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.h | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/refs.h b/refs.h
index a35eafc..122ec03 100644
--- a/refs.h
+++ b/refs.h
@@ -15,13 +15,23 @@ struct ref_lock {
 #define REF_ISBROKEN 0x04
 
 /*
- * Calls the specified function for each ref file until it returns
- * nonzero, and returns the value.  Please note that it is not safe to
- * modify references while an iteration is in progress, unless the
- * same callback function invocation that modifies the reference also
- * returns a nonzero value to immediately stop the iteration.
+ * The signature for the callback function for the for_each_*()
+ * functions below.  The memory pointed to by the refname and sha1
+ * arguments is only guaranteed to be valid for the duration of a
+ * single callback invocation.
+ */
+typedef int each_ref_fn(const char *refname,
+			const unsigned char *sha1, int flags, void *cb_data);
+
+/*
+ * The following functions invoke the specified callback function for
+ * each reference indicated.  If the function ever returns a nonzero
+ * value, stop the iteration and return that value.  Please note that
+ * it is not safe to modify references while an iteration is in
+ * progress, unless the same callback function invocation that
+ * modifies the reference also returns a nonzero value to immediately
+ * stop the iteration.
  */
-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
 extern int head_ref(each_ref_fn, void *);
 extern int for_each_ref(each_ref_fn, void *);
 extern int for_each_ref_in(const char *, each_ref_fn, void *);
-- 
1.8.2.3
Previous: Philip OakleyNext: Junio C Hamano
Message 37 of 40 in “Remove assumptions about each_ref_fn arg lifetimes”
  1. 00/25 Remove assumptions about each_ref_fn arg lifetimesMichael Haggerty, May 25, 2013
  2. 01/25 describe: make own copy of refnameMichael Haggerty, May 25, 2013
  3. 02/25 fetch: make own copies of refnamesMichael Haggerty, May 25, 2013
  4. 03/25 add_rev_cmdline(): make a copy of the name argumentMichael Haggerty, May 25, 2013
  5. 04/25 builtin_diff_tree(): make it obvious that function wants two entriesMichael Haggerty, May 25, 2013
  6. 05/25 cmd_diff(): use an object_array for holding treesMichael Haggerty, May 25, 2013
  7. 06/25 cmd_diff(): rename local variable "list" -> "entry"Michael Haggerty, May 25, 2013
  8. 07/25 cmd_diff(): make it obvious which cases are exclusive of each otherMichael Haggerty, May 25, 2013
  9. 08/25 revision: split some overly-long linesMichael Haggerty, May 25, 2013
  10. 09/25 object_array: add function object_array_filter()Michael Haggerty, May 25, 2013
  11. 10/25 revision: use object_array_filter() in implementation of gc_boundary()Michael Haggerty, May 25, 2013
  12. 11/25 object_array_remove_duplicates(): rewrite to reduce copyingMichael Haggerty, May 25, 2013
  13. Junio C HamanoMay 29, 2013
  14. Michael HaggertyMay 30, 2013
  15. Junio C HamanoJun 2, 2013
  16. 12/25 fsck: don't put a void*-shaped peg in a char*-shaped holeMichael Haggerty, May 25, 2013
  17. 13/25 find_first_merges(): initialize merges variable using initializerMichael Haggerty, May 25, 2013
  18. 14/25 find_first_merges(): remove unnecessary codeMichael Haggerty, May 25, 2013
  19. 15/25 object_array_entry: fix memory handling of the name fieldMichael Haggerty, May 25, 2013
  20. Junio C HamanoMay 29, 2013
  21. 16/25 do_fetch(): reduce scope of peer_itemMichael Haggerty, May 25, 2013
  22. 17/25 do_fetch(): clean up existing_refs before exitingMichael Haggerty, May 25, 2013
  23. 18/25 add_existing(): do not retain a reference to sha1Michael Haggerty, May 25, 2013
  24. 19/25 show_head_ref(): do not shadow name of argumentMichael Haggerty, May 25, 2013
  25. 20/25 show_head_ref(): rename first parameter to "refname"Michael Haggerty, May 25, 2013
  26. 21/25 string_list_add_one_ref(): rename first parameter to "refname"Michael Haggerty, May 25, 2013
  27. 22/25 string_list_add_refs_by_glob(): add a comment about memory managementMichael Haggerty, May 25, 2013
  28. Thomas RastMay 29, 2013
  29. Michael HaggertyMay 30, 2013
  30. 22/25 fixup! string_list_add_refs_by_glob(): add a comment about memory managementMichael Haggerty, May 30, 2013
  31. Junio C HamanoJun 3, 2013
  32. 23/25 exclude_existing(): set existing_refs.strdup_stringsMichael Haggerty, May 25, 2013
  33. 24/25 register_ref(): make a copy of the bad reference SHA-1Michael Haggerty, May 25, 2013
  34. Junio C HamanoMay 29, 2013
  35. Michael HaggertyMay 30, 2013
  36. Philip OakleyMay 30, 2013
  37. 25/25 refs: document the lifetime of the args passed to each_ref_fnMichael Haggerty, May 25, 2013
  38. Junio C HamanoMay 29, 2013
  39. Thomas RastMay 29, 2013
  40. Michael HaggertyMay 30, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.