{"thread":{"id":"24574","subject":"[PATCH] Remove useless temporary integer in builtin/push.c","startedAt":"2010-07-29T15:59:23Z","lastAt":"2010-08-02T18:51:59Z","messageCount":6,"participants":["Jared Hance","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"146718","messageId":"70ee84752cb7db08c65c608a12ed321dd2c26830.1280419073.git.jaredhance@gmail.com","threadId":"24574","inReplyTo":null,"subject":"[PATCH] Remove useless temporary integer in builtin/push.c","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-29T15:59:23Z","receivedAt":"2010-07-29T15:59:23Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Creating a variable nr here to use throughout the function only to change\nrefspec_nr to nr at the end, having not used refspec_nr the entire time,\nis rather pointless. Instead, simply increment refspec_nr.\n\nSigned-off-by: Jared Hance \n---\n builtin/push.c | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f4358b9..79d8192 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -25,10 +25,9 @@ static int refspec_nr;\n \n static void add_refspec(const char *ref)\n {\n-\tint nr = refspec_nr + 1;\n-\trefspec = xrealloc(refspec, nr * sizeof(char *));\n-\trefspec[nr-1] = ref;\n-\trefspec_nr = nr;\n+\trefspec_nr++;\n+\trefspec = xrealloc(refspec, refspec_nr * sizeof(char *));\n+\trefspec[refspec_nr-1] = ref;\n }\n \n static void set_refspecs(const char **refs, int nr)\n-- \n1.7.2\n"},{"id":"146741","messageId":"201007300021.34061.trast@student.ethz.ch","threadId":"24574","inReplyTo":"70ee84752cb7db08c65c608a12ed321dd2c26830.1280419073.git.jaredhance@gmail.com","subject":"Re: [PATCH] Remove useless temporary integer in builtin/push.c","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-07-29T22:21:33Z","receivedAt":"2010-07-29T22:21:33Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jared Hance wrote:\n> Creating a variable nr here to use throughout the function only to change\n> refspec_nr to nr at the end, having not used refspec_nr the entire time,\n> is rather pointless. Instead, simply increment refspec_nr.\n> \n> Signed-off-by: Jared Hance \n[...]\n> -\tint nr = refspec_nr + 1;\n> -\trefspec = xrealloc(refspec, nr * sizeof(char *));\n> -\trefspec[nr-1] = ref;\n> -\trefspec_nr = nr;\n> +\trefspec_nr++;\n> +\trefspec = xrealloc(refspec, refspec_nr * sizeof(char *));\n> +\trefspec[refspec_nr-1] = ref;\n\nWhile you're already here, you could switch to ALLOC_GROW instead to\navoid the n**2 behaviour of xrealloc...\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"146857","messageId":"cover.1280580026.git.jaredhance@gmail.com","threadId":"24574","inReplyTo":"201007300021.34061.trast@student.ethz.ch","subject":"[PATCH 0/2] Clean up add_refspec in builtin/push.c","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-31T12:54:32Z","receivedAt":"2010-07-31T12:54:32Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"This patch series removes code duplication and other poor coding\npractices from builtin/push.c.\n\nJared Hance (2):\n Remove useless temporary integer in builtin/push.c\n Use ALLOC_GROW in builtin/push.c\n\n builtin/push.c | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\n-- \n1.7.2\n"},{"id":"146858","messageId":"70ee84752cb7db08c65c608a12ed321dd2c26830.1280580026.git.jaredhance@gmail.com","threadId":"24574","inReplyTo":"cover.1280580026.git.jaredhance@gmail.com","subject":"[PATCH 1/2] Remove useless temporary integer in builtin/push.c","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-31T12:54:55Z","receivedAt":"2010-07-31T12:54:55Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"Creating a variable nr here to use throughout the function only to change\nrefspec_nr to nr at the end, having not used refspec_nr the entire time,\nis rather pointless. Instead, simply increment refspec_nr.\n\nSigned-off-by: Jared Hance \n---\n builtin/push.c | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f4358b9..79d8192 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -25,10 +25,9 @@ static int refspec_nr;\n \n static void add_refspec(const char *ref)\n {\n-\tint nr = refspec_nr + 1;\n-\trefspec = xrealloc(refspec, nr * sizeof(char *));\n-\trefspec[nr-1] = ref;\n-\trefspec_nr = nr;\n+\trefspec_nr++;\n+\trefspec = xrealloc(refspec, refspec_nr * sizeof(char *));\n+\trefspec[refspec_nr-1] = ref;\n }\n \n static void set_refspecs(const char **refs, int nr)\n-- \n1.7.2\n"},{"id":"146859","messageId":"fbf6f49fa7c1d6b265d6cf6cfa772ec133550389.1280580026.git.jaredhance@gmail.com","threadId":"24574","inReplyTo":"cover.1280580026.git.jaredhance@gmail.com","subject":"[PATCH 2/2] Use ALLOC_GROW in builtin/push.c","fromName":"Jared Hance","fromEmail":"jaredhance@gmail.com","sentAt":"2010-07-31T12:57:36Z","receivedAt":"2010-07-31T12:57:36Z","isPatch":true,"sender":{"key":"jaredhance@gmail.com","avatar":"https://avatars.githubusercontent.com/u/170192?v=4"},"body":"The current implementation of add_refspec(const char *ref) duplicates\nfunctionality found in the xalloc api. Use ALLOC_GROW instead to prevent\ncode duplication.\n\nSigned-off-by: Jared Hance \n---\n builtin/push.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 79d8192..0da0ec8 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -22,11 +22,12 @@ static int progress;\n \n static const char **refspec;\n static int refspec_nr;\n+static size_t refspec_alloc;\n \n static void add_refspec(const char *ref)\n {\n \trefspec_nr++;\n-\trefspec = xrealloc(refspec, refspec_nr * sizeof(char *));\n+\tALLOC_GROW(refspec, refspec_nr, refspec_alloc);\n \trefspec[refspec_nr-1] = ref;\n }\n \n-- \n1.7.2\n"},{"id":"146970","messageId":"7vocdklueo.fsf@alter.siamese.dyndns.org","threadId":"24574","inReplyTo":"70ee84752cb7db08c65c608a12ed321dd2c26830.1280580026.git.jaredhance@gmail.com","subject":"Re: [PATCH 1/2] Remove useless temporary integer in builtin/push.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T18:51:59Z","receivedAt":"2010-08-02T18:51:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jared Hance writes:\n\n> Creating a variable nr here to use throughout the function only to change\n> refspec_nr to nr at the end, having not used refspec_nr the entire time,\n> is rather pointless. Instead, simply increment refspec_nr.\n\nThat is something a compiler can notice and optimize out, so it byitself\nis not a good criteria to judge this change. The real issue is if the use\nof temporary makes the code easier to read or harder.\n\nWith the two patches squashed together to use ALLOC_GROW(), the result\nconforms to the pattern many codepaths use, and that makes it easier to\nread.\n\nWill queue, with these two squashed into one commit.\n\nThanks.\n\n> Signed-off-by: Jared Hance \n> ---\n> builtin/push.c | 7 +++----\n> 1 files changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index f4358b9..79d8192 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -25,10 +25,9 @@ static int refspec_nr;\n> \n> static void add_refspec(const char *ref)\n> {\n> -\tint nr = refspec_nr + 1;\n> -\trefspec = xrealloc(refspec, nr * sizeof(char *));\n> -\trefspec[nr-1] = ref;\n> -\trefspec_nr = nr;\n> +\trefspec_nr++;\n> +\trefspec = xrealloc(refspec, refspec_nr * sizeof(char *));\n> +\trefspec[refspec_nr-1] = ref;\n> }\n> \n> static void set_refspecs(const char **refs, int nr)\n> -- \n> 1.7.2\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"}]}