Re: [PATCH] sha1_name: pass object name length to diagnose_invalid_sha1_path()

2013-03-17 Thread Junio C Hamano 
René Scharfe rene.scha...@lsrfire.ath.cx writes:

 The only caller of diagnose_invalid_sha1_path() extracts a substring from
 an object name by creating a NUL-terminated copy of the interesting part.
 Add a length parameter to the function and thus avoid the need for an
 allocation, thereby simplifying the code.

 Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
 ---
  sha1_name.c | 32 ++--
  1 file changed, 14 insertions(+), 18 deletions(-)

 diff --git a/sha1_name.c b/sha1_name.c
 index 95003c7..4cea6d3 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -1137,7 +1137,8 @@ int get_sha1_blob(const char *name, unsigned char *sha1)
  static void diagnose_invalid_sha1_path(const char *prefix,
  const char *filename,
  const unsigned char *tree_sha1,
 -const char *object_name)
 +const char *object_name,
 +int object_name_len)
  {
   struct stat st;
   unsigned char sha1[20];
 @@ -1147,8 +1148,8 @@ static void diagnose_invalid_sha1_path(const char 
 *prefix,
   prefix = ;
  
   if (!lstat(filename, st))
 - die(Path '%s' exists on disk, but not in '%s'.,
 - filename, object_name);
 + die(Path '%s' exists on disk, but not in '%.*s'.,
 + filename, object_name_len, object_name);
   if (errno == ENOENT || errno == ENOTDIR) {
   char *fullname = xmalloc(strlen(filename)
+ strlen(prefix) + 1);
 @@ -1158,16 +1159,16 @@ static void diagnose_invalid_sha1_path(const char 
 *prefix,
   if (!get_tree_entry(tree_sha1, fullname,
   sha1, mode)) {
   die(Path '%s' exists, but not '%s'.\n
 - Did you mean '%s:%s' aka '%s:./%s'?,
 + Did you mean '%.*s:%s' aka '.*%.*s:./%s'?,

This is so unlike what I call Scharfe patch, which I can apply
with my eyes closed and expect everything to be perfect.

Other than that, I see this as a usual Scharfe patch ;-)  Will
squash an obvious fix in and apply.

Thanks.

   fullname,
   filename,
 - object_name,
 + object_name_len, object_name,
   fullname,
 - object_name,
 + object_name_len, object_name,
   filename);
   }
 - die(Path '%s' does not exist in '%s',
 - filename, object_name);
 + die(Path '%s' does not exist in '%.*s',
 + filename, object_name_len, object_name);
   }
  }
  
 @@ -1332,13 +1333,8 @@ static int get_sha1_with_context_1(const char *name,
   }
   if (*cp == ':') {
   unsigned char tree_sha1[20];
 - char *object_name = NULL;
 - if (only_to_die) {
 - object_name = xmalloc(cp-name+1);
 - strncpy(object_name, name, cp-name);
 - object_name[cp-name] = '\0';
 - }
 - if (!get_sha1_1(name, cp-name, tree_sha1, GET_SHA1_TREEISH)) {
 + int len = cp - name;
 + if (!get_sha1_1(name, len, tree_sha1, GET_SHA1_TREEISH)) {
   const char *filename = cp+1;
   char *new_filename = NULL;
  
 @@ -1348,8 +1344,8 @@ static int get_sha1_with_context_1(const char *name,
   ret = get_tree_entry(tree_sha1, filename, sha1, 
 oc-mode);
   if (ret  only_to_die) {
   diagnose_invalid_sha1_path(prefix, filename,
 -tree_sha1, 
 object_name);
 - free(object_name);
 +tree_sha1,
 +name, len);
   }
   hashcpy(oc-tree, tree_sha1);
   strncpy(oc-path, filename,
 @@ -1360,7 +1356,7 @@ static int get_sha1_with_context_1(const char *name,
   return ret;
   } else {
   if (only_to_die)
 - die(Invalid object name '%s'., object_name);
 + die(Invalid object name '%.*s'., len, name);
   }
   }
   return ret;
--
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] sha1_name: pass object name length to diagnose_invalid_sha1_path()

2013-03-17 Thread René Scharfe 

Am 17.03.2013 08:10, schrieb Junio C Hamano:

@@ -1158,16 +1159,16 @@ static void diagnose_invalid_sha1_path(const char 
*prefix,
if (!get_tree_entry(tree_sha1, fullname,
sha1, mode)) {
die(Path '%s' exists, but not '%s'.\n
-   Did you mean '%s:%s' aka '%s:./%s'?,
+   Did you mean '%.*s:%s' aka '.*%.*s:./%s'?,


Will squash an obvious fix in and apply.


Did I try to make a point there?  Certainly not.  It seems I need to go 
back to http://vim-adventures.com/.


Thank you for spotting this!
René

--
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] sha1_name: pass object name length to diagnose_invalid_sha1_path()

2013-03-16 Thread René Scharfe 
The only caller of diagnose_invalid_sha1_path() extracts a substring from
an object name by creating a NUL-terminated copy of the interesting part.
Add a length parameter to the function and thus avoid the need for an
allocation, thereby simplifying the code.

Signed-off-by: Rene Scharfe rene.scha...@lsrfire.ath.cx
---
 sha1_name.c | 32 ++--
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 95003c7..4cea6d3 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1137,7 +1137,8 @@ int get_sha1_blob(const char *name, unsigned char *sha1)
 static void diagnose_invalid_sha1_path(const char *prefix,
   const char *filename,
   const unsigned char *tree_sha1,
-  const char *object_name)
+  const char *object_name,
+  int object_name_len)
 {
struct stat st;
unsigned char sha1[20];
@@ -1147,8 +1148,8 @@ static void diagnose_invalid_sha1_path(const char *prefix,
prefix = ;
 
if (!lstat(filename, st))
-   die(Path '%s' exists on disk, but not in '%s'.,
-   filename, object_name);
+   die(Path '%s' exists on disk, but not in '%.*s'.,
+   filename, object_name_len, object_name);
if (errno == ENOENT || errno == ENOTDIR) {
char *fullname = xmalloc(strlen(filename)
 + strlen(prefix) + 1);
@@ -1158,16 +1159,16 @@ static void diagnose_invalid_sha1_path(const char 
*prefix,
if (!get_tree_entry(tree_sha1, fullname,
sha1, mode)) {
die(Path '%s' exists, but not '%s'.\n
-   Did you mean '%s:%s' aka '%s:./%s'?,
+   Did you mean '%.*s:%s' aka '.*%.*s:./%s'?,
fullname,
filename,
-   object_name,
+   object_name_len, object_name,
fullname,
-   object_name,
+   object_name_len, object_name,
filename);
}
-   die(Path '%s' does not exist in '%s',
-   filename, object_name);
+   die(Path '%s' does not exist in '%.*s',
+   filename, object_name_len, object_name);
}
 }
 
@@ -1332,13 +1333,8 @@ static int get_sha1_with_context_1(const char *name,
}
if (*cp == ':') {
unsigned char tree_sha1[20];
-   char *object_name = NULL;
-   if (only_to_die) {
-   object_name = xmalloc(cp-name+1);
-   strncpy(object_name, name, cp-name);
-   object_name[cp-name] = '\0';
-   }
-   if (!get_sha1_1(name, cp-name, tree_sha1, GET_SHA1_TREEISH)) {
+   int len = cp - name;
+   if (!get_sha1_1(name, len, tree_sha1, GET_SHA1_TREEISH)) {
const char *filename = cp+1;
char *new_filename = NULL;
 
@@ -1348,8 +1344,8 @@ static int get_sha1_with_context_1(const char *name,
ret = get_tree_entry(tree_sha1, filename, sha1, 
oc-mode);
if (ret  only_to_die) {
diagnose_invalid_sha1_path(prefix, filename,
-  tree_sha1, 
object_name);
-   free(object_name);
+  tree_sha1,
+  name, len);
}
hashcpy(oc-tree, tree_sha1);
strncpy(oc-path, filename,
@@ -1360,7 +1356,7 @@ static int get_sha1_with_context_1(const char *name,
return ret;
} else {
if (only_to_die)
-   die(Invalid object name '%s'., object_name);
+   die(Invalid object name '%.*s'., len, name);
}
}
return ret;
-- 
1.8.2

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