* [PATCH v2 0/4] Fix some constness errors in fetch-pack
@ 2012-05-21 7:59 mhagger
2012-05-21 7:59 ` [PATCH v2 1/4] cmd_fetch_pack(): declare dest to be const mhagger
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: mhagger @ 2012-05-21 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Fix some constness errors that I noticed while reading the code in
builtin/fetch-pack.c.
This version differs from v1 as follows:
* cmd_fetch_pack() is not made to free the memory that it allocates.
* The second patch is broken into separate pieces to make each piece
more readable.
Michael Haggerty (4):
cmd_fetch_pack(): declare dest to be const
cmd_fetch_pack(): handle non-option arguments outside of the loop
cmd_fetch_pack(): combine the loop termination conditions
cmd_fetch_pack(): respect constness of argv parameter
builtin/fetch-pack.c | 145 ++++++++++++++++++++++++--------------------------
1 file changed, 71 insertions(+), 74 deletions(-)
--
1.7.10
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/4] cmd_fetch_pack(): declare dest to be const
2012-05-21 7:59 [PATCH v2 0/4] Fix some constness errors in fetch-pack mhagger
@ 2012-05-21 7:59 ` mhagger
2012-05-21 7:59 ` [PATCH v2 2/4] cmd_fetch_pack(): handle non-option arguments outside of the loop mhagger
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: mhagger @ 2012-05-21 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
There is no need for it to be non-const, and this avoids the need
for casting away the constness of an argv element.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 10db15b..7e9d62f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -901,7 +901,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
{
int i, ret, nr_heads;
struct ref *ref = NULL;
- char *dest = NULL, **heads;
+ const char *dest = NULL;
+ char **heads;
int fd[2];
char *pack_lockfile = NULL;
char **pack_lockfile_ptr = NULL;
@@ -971,7 +972,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
}
usage(fetch_pack_usage);
}
- dest = (char *)arg;
+ dest = arg;
heads = (char **)(argv + i + 1);
nr_heads = argc - i - 1;
break;
@@ -1018,7 +1019,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
fd[0] = 0;
fd[1] = 1;
} else {
- conn = git_connect(fd, (char *)dest, args.uploadpack,
+ conn = git_connect(fd, dest, args.uploadpack,
args.verbose ? CONNECT_VERBOSE : 0);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/4] cmd_fetch_pack(): handle non-option arguments outside of the loop
2012-05-21 7:59 [PATCH v2 0/4] Fix some constness errors in fetch-pack mhagger
2012-05-21 7:59 ` [PATCH v2 1/4] cmd_fetch_pack(): declare dest to be const mhagger
@ 2012-05-21 7:59 ` mhagger
2012-05-21 7:59 ` [PATCH v2 3/4] cmd_fetch_pack(): combine the loop termination conditions mhagger
2012-05-21 7:59 ` [PATCH v2 4/4] cmd_fetch_pack(): respect constness of argv parameter mhagger
3 siblings, 0 replies; 5+ messages in thread
From: mhagger @ 2012-05-21 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This makes it more obvious that the code is always executed unless
there is an error, and that the first initialization of nr_heads is
unnecessary.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 7e9d62f..dabf5e9 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -910,7 +910,6 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
packet_trace_identity("fetch-pack");
- nr_heads = 0;
heads = NULL;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -972,14 +971,17 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
}
usage(fetch_pack_usage);
}
- dest = arg;
- heads = (char **)(argv + i + 1);
- nr_heads = argc - i - 1;
break;
}
- if (!dest)
+
+ if (i < argc)
+ dest = argv[i++];
+ else
usage(fetch_pack_usage);
+ heads = (char **)(argv + i);
+ nr_heads = argc - i;
+
if (args.stdin_refs) {
/*
* Copy refs from cmdline to new growable list, then
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/4] cmd_fetch_pack(): combine the loop termination conditions
2012-05-21 7:59 [PATCH v2 0/4] Fix some constness errors in fetch-pack mhagger
2012-05-21 7:59 ` [PATCH v2 1/4] cmd_fetch_pack(): declare dest to be const mhagger
2012-05-21 7:59 ` [PATCH v2 2/4] cmd_fetch_pack(): handle non-option arguments outside of the loop mhagger
@ 2012-05-21 7:59 ` mhagger
2012-05-21 7:59 ` [PATCH v2 4/4] cmd_fetch_pack(): respect constness of argv parameter mhagger
3 siblings, 0 replies; 5+ messages in thread
From: mhagger @ 2012-05-21 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
If an argument that does not start with '-' is found, the loop is
terminated. So move that check into the for-loop condition.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 113 ++++++++++++++++++++++++--------------------------
1 file changed, 55 insertions(+), 58 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index dabf5e9..96849a4 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -911,67 +911,64 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
packet_trace_identity("fetch-pack");
heads = NULL;
- for (i = 1; i < argc; i++) {
+ for (i = 1; i < argc && *argv[i] == '-'; i++) {
const char *arg = argv[i];
- if (*arg == '-') {
- if (!prefixcmp(arg, "--upload-pack=")) {
- args.uploadpack = arg + 14;
- continue;
- }
- if (!prefixcmp(arg, "--exec=")) {
- args.uploadpack = arg + 7;
- continue;
- }
- if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
- args.quiet = 1;
- continue;
- }
- if (!strcmp("--keep", arg) || !strcmp("-k", arg)) {
- args.lock_pack = args.keep_pack;
- args.keep_pack = 1;
- continue;
- }
- if (!strcmp("--thin", arg)) {
- args.use_thin_pack = 1;
- continue;
- }
- if (!strcmp("--include-tag", arg)) {
- args.include_tag = 1;
- continue;
- }
- if (!strcmp("--all", arg)) {
- args.fetch_all = 1;
- continue;
- }
- if (!strcmp("--stdin", arg)) {
- args.stdin_refs = 1;
- continue;
- }
- if (!strcmp("-v", arg)) {
- args.verbose = 1;
- continue;
- }
- if (!prefixcmp(arg, "--depth=")) {
- args.depth = strtol(arg + 8, NULL, 0);
- continue;
- }
- if (!strcmp("--no-progress", arg)) {
- args.no_progress = 1;
- continue;
- }
- if (!strcmp("--stateless-rpc", arg)) {
- args.stateless_rpc = 1;
- continue;
- }
- if (!strcmp("--lock-pack", arg)) {
- args.lock_pack = 1;
- pack_lockfile_ptr = &pack_lockfile;
- continue;
- }
- usage(fetch_pack_usage);
+ if (!prefixcmp(arg, "--upload-pack=")) {
+ args.uploadpack = arg + 14;
+ continue;
+ }
+ if (!prefixcmp(arg, "--exec=")) {
+ args.uploadpack = arg + 7;
+ continue;
+ }
+ if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
+ args.quiet = 1;
+ continue;
+ }
+ if (!strcmp("--keep", arg) || !strcmp("-k", arg)) {
+ args.lock_pack = args.keep_pack;
+ args.keep_pack = 1;
+ continue;
+ }
+ if (!strcmp("--thin", arg)) {
+ args.use_thin_pack = 1;
+ continue;
+ }
+ if (!strcmp("--include-tag", arg)) {
+ args.include_tag = 1;
+ continue;
+ }
+ if (!strcmp("--all", arg)) {
+ args.fetch_all = 1;
+ continue;
}
- break;
+ if (!strcmp("--stdin", arg)) {
+ args.stdin_refs = 1;
+ continue;
+ }
+ if (!strcmp("-v", arg)) {
+ args.verbose = 1;
+ continue;
+ }
+ if (!prefixcmp(arg, "--depth=")) {
+ args.depth = strtol(arg + 8, NULL, 0);
+ continue;
+ }
+ if (!strcmp("--no-progress", arg)) {
+ args.no_progress = 1;
+ continue;
+ }
+ if (!strcmp("--stateless-rpc", arg)) {
+ args.stateless_rpc = 1;
+ continue;
+ }
+ if (!strcmp("--lock-pack", arg)) {
+ args.lock_pack = 1;
+ pack_lockfile_ptr = &pack_lockfile;
+ continue;
+ }
+ usage(fetch_pack_usage);
}
if (i < argc)
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 4/4] cmd_fetch_pack(): respect constness of argv parameter
2012-05-21 7:59 [PATCH v2 0/4] Fix some constness errors in fetch-pack mhagger
` (2 preceding siblings ...)
2012-05-21 7:59 ` [PATCH v2 3/4] cmd_fetch_pack(): combine the loop termination conditions mhagger
@ 2012-05-21 7:59 ` mhagger
3 siblings, 0 replies; 5+ messages in thread
From: mhagger @ 2012-05-21 7:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
The old code cast away the constness of the strings passed to the
function in argument argv[], which could result in their being
modified by filter_refs(). Fix by copying reference names from argv
and putting them into our own array (similarly to how refnames passed
to stdin were already handled).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
This change results in heads being left set to NULL if nr_heads==0,
but all of the code paths are OK with this.
builtin/fetch-pack.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 96849a4..7ad9e54 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -899,10 +899,11 @@ static void fetch_pack_setup(void)
int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
{
- int i, ret, nr_heads;
+ int i, ret;
struct ref *ref = NULL;
const char *dest = NULL;
- char **heads;
+ int alloc_heads = 0, nr_heads = 0;
+ char **heads = NULL;
int fd[2];
char *pack_lockfile = NULL;
char **pack_lockfile_ptr = NULL;
@@ -910,7 +911,6 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
packet_trace_identity("fetch-pack");
- heads = NULL;
for (i = 1; i < argc && *argv[i] == '-'; i++) {
const char *arg = argv[i];
@@ -976,17 +976,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
else
usage(fetch_pack_usage);
- heads = (char **)(argv + i);
- nr_heads = argc - i;
-
+ /*
+ * Copy refs from cmdline to growable list, then append any
+ * refs from the standard input:
+ */
+ ALLOC_GROW(heads, argc - i, alloc_heads);
+ for (; i < argc; i++)
+ heads[nr_heads++] = xstrdup(argv[i]);
if (args.stdin_refs) {
- /*
- * Copy refs from cmdline to new growable list, then
- * append the refs from the standard input.
- */
- int alloc_heads = nr_heads;
- int size = nr_heads * sizeof(*heads);
- heads = memcpy(xmalloc(size), heads, size);
if (args.stateless_rpc) {
/* in stateless RPC mode we use pkt-line to read
* from stdin, until we get a flush packet
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-05-21 8:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-21 7:59 [PATCH v2 0/4] Fix some constness errors in fetch-pack mhagger
2012-05-21 7:59 ` [PATCH v2 1/4] cmd_fetch_pack(): declare dest to be const mhagger
2012-05-21 7:59 ` [PATCH v2 2/4] cmd_fetch_pack(): handle non-option arguments outside of the loop mhagger
2012-05-21 7:59 ` [PATCH v2 3/4] cmd_fetch_pack(): combine the loop termination conditions mhagger
2012-05-21 7:59 ` [PATCH v2 4/4] cmd_fetch_pack(): respect constness of argv parameter mhagger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.