Re: [PATCH v4 13/14] builtin/update-index.c: cleanup update_one

2013-11-08 Thread Karsten Blees
Am 07.11.2013 22:40, schrieb Junio C Hamano:
> Karsten Blees  writes:
> 
>> -if (p < path || p > path + strlen(path))
>> -free((char *)p);
>> +free(p);
> 
> The non-const cast was there for a reason.  I'll locally fix it up
> (there is another instance of the same) to get it compile before
> queuing it on 'pu'.
> 
> Thanks.
> 

Seems I spent too much time on code analysis and valgrind results that I missed 
the compiler warnings...didn't we have -Werror once?
--
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 v4 13/14] builtin/update-index.c: cleanup update_one

2013-11-07 Thread Junio C Hamano
Karsten Blees  writes:

> - if (p < path || p > path + strlen(path))
> - free((char *)p);
> + free(p);

The non-const cast was there for a reason.  I'll locally fix it up
(there is another instance of the same) to get it compile before
queuing it on 'pu'.

Thanks.

--
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 v4 13/14] builtin/update-index.c: cleanup update_one

2013-11-07 Thread Karsten Blees
do_reupdate calls update_one with a cache_entry.name, there's no need for
the extra sanitation / normalization that happens in prefix_path.
cmd_update_index calls update_one with an already prefixed path, no need to
prefix_path twice.

Remove the extra prefix_path from update_one. Also remove the now unused
'prefix' and 'prefix_length' parameters.

As of d089eba "setup: sanitize absolute and funny paths in get_pathspec()",
prefix_path uncoditionally returns a copy, even if the passed in path isn't
changed. Lets unconditionally free() the result.

Signed-off-by: Karsten Blees 
---
 builtin/update-index.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index d180d80..b654d27 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -274,36 +274,32 @@ static void chmod_path(int flip, const char *path)
die("git update-index: cannot chmod %cx '%s'", flip, path);
 }
 
-static void update_one(const char *path, const char *prefix, int prefix_length)
+static void update_one(const char *path)
 {
-   const char *p = prefix_path(prefix, prefix_length, path);
-   if (!verify_path(p)) {
+   if (!verify_path(path)) {
fprintf(stderr, "Ignoring path %s\n", path);
-   goto free_return;
+   return;
}
if (mark_valid_only) {
-   if (mark_ce_flags(p, CE_VALID, mark_valid_only == MARK_FLAG))
+   if (mark_ce_flags(path, CE_VALID, mark_valid_only == MARK_FLAG))
die("Unable to mark file %s", path);
-   goto free_return;
+   return;
}
if (mark_skip_worktree_only) {
-   if (mark_ce_flags(p, CE_SKIP_WORKTREE, mark_skip_worktree_only 
== MARK_FLAG))
+   if (mark_ce_flags(path, CE_SKIP_WORKTREE, 
mark_skip_worktree_only == MARK_FLAG))
die("Unable to mark file %s", path);
-   goto free_return;
+   return;
}
 
if (force_remove) {
-   if (remove_file_from_cache(p))
+   if (remove_file_from_cache(path))
die("git update-index: unable to remove %s", path);
report("remove '%s'", path);
-   goto free_return;
+   return;
}
-   if (process_path(p))
+   if (process_path(path))
die("Unable to process path %s", path);
report("add '%s'", path);
- free_return:
-   if (p < path || p > path + strlen(path))
-   free((char *)p);
 }
 
 static void read_index_info(int line_termination)
@@ -579,7 +575,7 @@ static int do_reupdate(int ac, const char **av,
 * or worse yet 'allow_replace', active_nr may decrease.
 */
save_nr = active_nr;
-   update_one(ce->name, NULL, 0);
+   update_one(ce->name);
if (save_nr != active_nr)
goto redo;
}
@@ -836,11 +832,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
 
setup_work_tree();
p = prefix_path(prefix, prefix_length, path);
-   update_one(p, NULL, 0);
+   update_one(p);
if (set_executable_bit)
chmod_path(set_executable_bit, p);
-   if (p < path || p > path + strlen(path))
-   free((char *)p);
+   free(p);
ctx.argc--;
ctx.argv++;
break;
@@ -879,11 +874,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
strbuf_swap(&buf, &nbuf);
}
p = prefix_path(prefix, prefix_length, buf.buf);
-   update_one(p, NULL, 0);
+   update_one(p);
if (set_executable_bit)
chmod_path(set_executable_bit, p);
-   if (p < buf.buf || p > buf.buf + buf.len)
-   free((char *)p);
+   free(p);
}
strbuf_release(&nbuf);
strbuf_release(&buf);
-- 
1.8.4.msysgit.0.12.g88f5ed0

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