Re: [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
On 09/17/2014 12:28 AM, Jonathan Nieder wrote: Michael Haggerty wrote: After commit_lock_file() is called, then the lock_file object is necessarily either committed or rolled back. So there is no need to call rollback_lock_file() again in either of these cases. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu This seems to involve an unadvertised behavior change: before it wouldn't call git_config_clear() on failure, and after the patch it would. Intended? I think it would be clearer with the goto for exception handling maintained (and just a second 'lock = NULL' assignment). Good catch; will fix. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
After commit_lock_file() is called, then the lock_file object is
necessarily either committed or rolled back. So there is no need to
call rollback_lock_file() again in either of these cases.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
config.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/config.c b/config.c
index 83c913a..a8ab809 100644
--- a/config.c
+++ b/config.c
@@ -2076,17 +2076,17 @@ int git_config_set_multivar_in_file(const char
*config_filename,
if (commit_lock_file(lock) 0) {
error(could not commit config file %s, config_filename);
ret = CONFIG_NO_WRITE;
- goto out_free;
- }
+ } else
+ ret = 0;
/*
-* lock is committed, so don't try to roll it back below.
-* NOTE: Since lockfile.c keeps a linked list of all created
-* lock_file structures, it isn't safe to free(lock). It's
-* better to just leave it hanging around.
+* lock is committed or rolled back now, so there is no need
+* to roll it back below. NOTE: Since lockfile.c keeps a
+* linked list of all created lock_file structures, it isn't
+* safe to free(lock). We have to just leave it hanging
+* around.
*/
lock = NULL;
- ret = 0;
/* Invalidate the config cache */
git_config_clear();
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 22/35] git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
Michael Haggerty wrote: After commit_lock_file() is called, then the lock_file object is necessarily either committed or rolled back. So there is no need to call rollback_lock_file() again in either of these cases. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu This seems to involve an unadvertised behavior change: before it wouldn't call git_config_clear() on failure, and after the patch it would. Intended? I think it would be clearer with the goto for exception handling maintained (and just a second 'lock = NULL' assignment). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
