Re: [PATCH v2 4/7] struct ref_lock: delete the force_write member
On 03/02/2015 10:44 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
Instead, compute the value when it is needed.
@@ -2318,8 +2317,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
ref_file = git_path(%s, refname);
-if ((flags REF_NODEREF) (type REF_ISSYMREF))
-lock-force_write = 1;
retry:
switch (safe_create_leading_directories(ref_file)) {
@@ -3787,8 +3784,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
-if (!update-lock-force_write
-!hashcmp(update-lock-old_sha1, update-new_sha1))
{
+if (!((update-type REF_ISSYMREF)
+ (update-flags REF_NODEREF))
+ !hashcmp(update-lock-old_sha1,
update-new_sha1)) {
+/*
+ * The reference already has the desired
+ * value, so we don't need to write it.
+ */
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
The code before and after the change are equivalent.
It shouldn't be the case, but somehow I find the original slightly
easier to understand. [...]
I had the same feeling; thanks for confirming it. How about I introduce
a temporary variable `overwriting_symref` as an aid to the reader? I
think this makes it pretty clear:
if (!is_null_sha1(update-new_sha1)) {
- if (!update-lock-force_write
- !hashcmp(update-lock-old_sha1, update-new_sha1))
{
+ int overwriting_symref = ((update-type REF_ISSYMREF)
+ (update-flags
REF_NODEREF));
+
+ if (!overwriting_symref
+ !hashcmp(update-lock-old_sha1,
update-new_sha1)) {
+ /*
+ * The reference already has the desired
+ * value, so we don't need to write it.
+ */
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
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
Re: [PATCH v2 4/7] struct ref_lock: delete the force_write member
Michael Haggerty mhag...@alum.mit.edu writes:
Instead, compute the value when it is needed.
@@ -2318,8 +2317,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
ref_file = git_path(%s, refname);
- if ((flags REF_NODEREF) (type REF_ISSYMREF))
- lock-force_write = 1;
retry:
switch (safe_create_leading_directories(ref_file)) {
@@ -3787,8 +3784,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- if (!update-lock-force_write
- !hashcmp(update-lock-old_sha1, update-new_sha1))
{
+ if (!((update-type REF_ISSYMREF)
+(update-flags REF_NODEREF))
+ !hashcmp(update-lock-old_sha1,
update-new_sha1)) {
+ /*
+ * The reference already has the desired
+ * value, so we don't need to write it.
+ */
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
The code before and after the change are equivalent.
It shouldn't be the case, but somehow I find the original slightly
easier to understand. The before and after says the same thing,
i.e. the code used to be:
- We say do the write-out without questioning when we are
updating a symbolic ref without dereferencing.
- Do nothing and unlock if we are not told to do the write-out
without questioning and the update will be a no-op anyway.
while the code after the change says:
+ Do nothing and unlock if we are not handling update a symbolic
ref without dereferencing and the update will be a no-op anyway.
Perhaps the former has the same effect as avoid a single complex
sentence and use two short sentences instead.
The negation in the condition does not help, either.
* If we are updating a symbolic ref without dereferencing, or if we
are updating with a different object name, we definitely have to
write.
would be easier to understand, perhaps? I.e.
if (hashcmp(update-lock-old_sha1, update-lock-new_sha1) ||
((update-type REF_ISSYMREF) (update-flags REF_NO_DEREF))) {
/* do the write-out thing */
} else {
/* the request to update from the same to the same is a no-op */
unlock_ref(update-lock);
update-lock = NULL;
}
I dunno.
--
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 v2 4/7] struct ref_lock: delete the force_write member
From: Stefan Beller sbel...@google.com
Instead, compute the value when it is needed.
Signed-off-by: Stefan Beller sbel...@google.com
Edited-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 3ed9ea6..f2e9883 100644
--- a/refs.c
+++ b/refs.c
@@ -12,7 +12,6 @@ struct ref_lock {
struct lock_file *lk;
unsigned char old_sha1[20];
int lock_fd;
- int force_write;
};
/*
@@ -2318,8 +2317,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
lock-ref_name = xstrdup(refname);
lock-orig_ref_name = xstrdup(orig_refname);
ref_file = git_path(%s, refname);
- if ((flags REF_NODEREF) (type REF_ISSYMREF))
- lock-force_write = 1;
retry:
switch (safe_create_leading_directories(ref_file)) {
@@ -3787,8 +3784,13 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- if (!update-lock-force_write
- !hashcmp(update-lock-old_sha1, update-new_sha1))
{
+ if (!((update-type REF_ISSYMREF)
+ (update-flags REF_NODEREF))
+!hashcmp(update-lock-old_sha1,
update-new_sha1)) {
+ /*
+* The reference already has the desired
+* value, so we don't need to write it.
+*/
unlock_ref(update-lock);
update-lock = NULL;
} else if (write_ref_sha1(update-lock,
update-new_sha1,
--
2.1.4
--
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
