Re: [PATCH v11 36/41] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-06-03 Thread Ronnie Sahlberg 
On Fri, May 30, 2014 at 11:12 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Ronnie Sahlberg wrote:

 Move the check for check_refname_format from lock_any_ref_for_update
 to lock_ref_sha1_basic. At some later stage we will get rid of
 lock_any_ref_for_update completely.

 Do you know if this will cause any functional change?

 What is the potential impact?  Is that impact worth it?  (Given how
 broken the recovery codepaths currently are, I suspect this change is
 worth it, but it seems worth documenting in the log message.)

Thanks.

Updated the commit message to mention this.

  This changes semantics for lock_ref_sha1_basic slightly. With this change
  it is no longer possible to open a ref that has a badly name which breaks
  any codepaths that tries to open and repair badly named refs. The normal refs
  API should not allow neither creating nor accessing refs with invalid names.
  If we need such recovery code we could add it as an option to git
fsck and have
  git fsck be the only sanctioned way of bypassing the normal API and checks.


 Thanks,
 Jonathan
--
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 v11 36/41] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-30 Thread Jonathan Nieder 
Ronnie Sahlberg wrote:

 Move the check for check_refname_format from lock_any_ref_for_update
 to lock_ref_sha1_basic. At some later stage we will get rid of
 lock_any_ref_for_update completely.

Do you know if this will cause any functional change?

What is the potential impact?  Is that impact worth it?  (Given how
broken the recovery codepaths currently are, I suspect this change is
worth it, but it seems worth documenting in the log message.)

Thanks,
Jonathan
--
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 v11 36/41] refs.c: move the check for valid refname to lock_ref_sha1_basic

2014-05-27 Thread Ronnie Sahlberg 
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.

This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 08dde5b..2952871 100644
--- a/refs.c
+++ b/refs.c
@@ -2043,6 +2043,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
int missing = 0;
int attempts_remaining = 3;
 
+   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
+   return NULL;
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
 
@@ -2134,8 +2137,6 @@ struct ref_lock *lock_any_ref_for_update(const char 
*refname,
 const unsigned char *old_sha1,
 int flags, int *type_p)
 {
-   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
-   return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
 }
 
-- 
2.0.0.rc3.474.g0203784

--
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