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

[PATCH v4 2/4] refs/files-backend: fix memory leak in lock_ref_for_update

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Sep 9, 2017, 06:57 UTC
Message-ID
<f06c7cab8e335f5459388d20776c19d5384ebc29.1504939825.git.martin.agren@gmail.com>
In-Reply-To
<cover.1504939825.git.martin.agren@gmail.com>

After the previous patch, none of the functions we call hold on to `referent.buf`, so we can safely release the string buffer before returning.

Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 refs/files-backend.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index baceef3b3..3d6363966 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2756,7 +2756,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 	struct strbuf referent = STRBUF_INIT;
 	int mustexist = (update->flags & REF_HAVE_OLD) &&
 		!is_null_oid(&update->old_oid);
-	int ret;
+	int ret = 0;
 	struct ref_lock *lock;
 
 	files_assert_main_repository(refs, "lock_ref_for_update");
@@ -2768,7 +2768,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		ret = split_head_update(update, transaction, head_ref,
 					affected_refnames, err);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
 	ret = lock_raw_ref(refs, update->refname, mustexist,
@@ -2782,7 +2782,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		strbuf_addf(err, "cannot lock ref '%s': %s",
 			    original_update_refname(update), reason);
 		free(reason);
-		return ret;
+		goto out;
 	}
 
 	update->backend_data = lock;
@@ -2801,10 +2801,12 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 					strbuf_addf(err, "cannot lock ref '%s': "
 						    "error reading reference",
 						    original_update_refname(update));
-					return -1;
+					ret = -1;
+					goto out;
 				}
 			} else if (check_old_oid(update, &lock->old_oid, err)) {
-				return TRANSACTION_GENERIC_ERROR;
+				ret = TRANSACTION_GENERIC_ERROR;
+				goto out;
 			}
 		} else {
 			/*
@@ -2818,13 +2820,15 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 						  referent.buf, transaction,
 						  affected_refnames, err);
 			if (ret)
-				return ret;
+				goto out;
 		}
 	} else {
 		struct ref_update *parent_update;
 
-		if (check_old_oid(update, &lock->old_oid, err))
-			return TRANSACTION_GENERIC_ERROR;
+		if (check_old_oid(update, &lock->old_oid, err)) {
+			ret = TRANSACTION_GENERIC_ERROR;
+			goto out;
+		}
 
 		/*
 		 * If this update is happening indirectly because of a
@@ -2861,7 +2865,8 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 				    "cannot update ref '%s': %s",
 				    update->refname, write_err);
 			free(write_err);
-			return TRANSACTION_GENERIC_ERROR;
+			ret = TRANSACTION_GENERIC_ERROR;
+			goto out;
 		} else {
 			update->flags |= REF_NEEDS_COMMIT;
 		}
@@ -2875,10 +2880,14 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 		if (close_ref(lock)) {
 			strbuf_addf(err, "couldn't close '%s.lock'",
 				    update->refname);
-			return TRANSACTION_GENERIC_ERROR;
+			ret = TRANSACTION_GENERIC_ERROR;
+			goto out;
 		}
 	}
-	return 0;
+
+out:
+	strbuf_release(&referent);
+	return ret;
 }
 
 /*
-- 
2.14.1.460.g848a19d64
Previous: Martin ÅgrenNext: Martin Ågren
Message 24 of 34 in “refs/files-backend: duplicate strings added to affected_refnames”
  1. 1/2 refs/files-backend: duplicate strings added to affected_refnamesMartin Ågren, Aug 25, 2017
  2. 2/2 refs/files-backend: fix memory leak in lock_ref_for_updateMartin Ågren, Aug 25, 2017
  3. Junio C HamanoAug 25, 2017
  4. Martin ÅgrenAug 26, 2017
  5. Michael HaggertyAug 28, 2017
  6. Martin ÅgrenAug 28, 2017
  7. 1/2 refs/files-backend: add longer-scoped copy of string to listMartin Ågren, Aug 28, 2017
  8. Michael HaggertyAug 29, 2017
  9. 2/2 refs/files-backend: fix memory leak in lock_ref_for_updateMartin Ågren, Aug 28, 2017
  10. Michael HaggertyAug 29, 2017
  11. Martin ÅgrenAug 29, 2017
  12. 1/3 refs/files-backend: add longer-scoped copy of string to listMartin Ågren, Aug 29, 2017
  13. Michael HaggertyAug 30, 2017
  14. Martin ÅgrenAug 30, 2017
  15. Junio C HamanoSep 5, 2017
  16. Martin ÅgrenSep 5, 2017
  17. Jeff KingSep 5, 2017
  18. Junio C HamanoSep 5, 2017
  19. Martin ÅgrenSep 6, 2017
  20. Junio C HamanoSep 6, 2017
  21. Jeff KingSep 6, 2017
  22. 0/4 Rerolling ma/split-symref-update-fixMartin Ågren, Sep 9, 2017
  23. 1/4 refs/files-backend: add longer-scoped copy of string to listMartin Ågren, Sep 9, 2017
  24. 2/4 refs/files-backend: fix memory leak in lock_ref_for_updateMartin Ågren, Sep 9, 2017
  25. 3/4 refs/files-backend: correct return value in lock_ref_for_updateMartin Ågren, Sep 9, 2017
  26. 4/4 refs/files-backend: add `refname`, not "HEAD", to listMartin Ågren, Sep 9, 2017
  27. Jeff KingSep 9, 2017
  28. Jeff KingSep 5, 2017
  29. Michael HaggertySep 5, 2017
  30. Jeff KingSep 5, 2017
  31. 2/3 refs/files-backend: fix memory leak in lock_ref_for_updateMartin Ågren, Aug 29, 2017
  32. Jeff KingSep 5, 2017
  33. Martin ÅgrenSep 5, 2017
  34. 3/3 refs/files-backend: correct return value in lock_ref_for_updateMartin Ågren, Aug 29, 2017

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.