# [PATCH] Avoid recalculating filename string pointer. 4 messages from 2007-11-22 to 2007-11-25. Participants: André Goddard Rosa, Mike Hommey, Junio C Hamano. Thread: https://gitlist.dev/t/10969 ## André Goddard Rosa, 2007-11-22 00:59 Subject: [PATCH] Avoid recalculating filename string pointer. Message-ID: URL: https://gitlist.dev/e/b8bf37780711211659i65a99493te3e3d5cee008ae7d%40mail.gmail.com ``` Hi, all! Please cc: me as I'm not subscribed. I'm sending the patch inline only for review, probably it is mangled. Please use the attached patch if you agree with it. Sorry about sending it attached. >From b6b05d9f8d8e053df4e971cd229e03b778c4d163 Mon Sep 17 00:00:00 2001 From: Andre Goddard Rosa Date: Tue, 27 Nov 2007 10:17:54 -0200 Subject: [PATCH] Avoid recalculating filename string pointer. Signed-off-by: Andre Goddard Rosa --- fast-import.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fast-import.c b/fast-import.c index 98c2bd5..2d262eb 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) else if (!prefixcmp(a, "--export-marks=")) mark_file = a + 15; else if (!prefixcmp(a, "--export-pack-edges=")) { + char *filename = a + 20; + if (pack_edges) fclose(pack_edges); - pack_edges = fopen(a + 20, "a"); + pack_edges = fopen(filename, "a"); if (!pack_edges) - die("Cannot open %s: %s", a + 20, strerror(errno)); + die("Cannot open %s: %s", filename, strerror(errno)); } else if (!strcmp(a, "--force")) force_update = 1; else if (!strcmp(a, "--quiet")) -- 1.5.3.6.861.gd794-dirty From b6b05d9f8d8e053df4e971cd229e03b778c4d163 Mon Sep 17 00:00:00 2001 From: Andre Goddard Rosa Date: Tue, 27 Nov 2007 10:17:54 -0200 Subject: [PATCH] Avoid recalculating filename string pointer. Signed-off-by: Andre Goddard Rosa --- fast-import.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fast-import.c b/fast-import.c index 98c2bd5..2d262eb 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) else if (!prefixcmp(a, "--export-marks=")) mark_file = a + 15; else if (!prefixcmp(a, "--export-pack-edges=")) { + char *filename = a + 20; + if (pack_edges) fclose(pack_edges); - pack_edges = fopen(a + 20, "a"); + pack_edges = fopen(filename, "a"); if (!pack_edges) - die("Cannot open %s: %s", a + 20, strerror(errno)); + die("Cannot open %s: %s", filename, strerror(errno)); } else if (!strcmp(a, "--force")) force_update = 1; else if (!strcmp(a, "--quiet")) -- 1.5.3.6.861.gd794-dirty ``` ## Mike Hommey, 2007-11-22 19:54 Subject: Re: [PATCH] Avoid recalculating filename string pointer. Message-ID: <20071122195457.GB19675@glandium.org> URL: https://gitlist.dev/e/20071122195457.GB19675%40glandium.org In-Reply-To: ``` On Wed, Nov 21, 2007 at 10:59:41PM -0200, André Goddard Rosa wrote: > --- a/fast-import.c > +++ b/fast-import.c > @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) > else if (!prefixcmp(a, "--export-marks=")) > mark_file = a + 15; > else if (!prefixcmp(a, "--export-pack-edges=")) { > + char *filename = a + 20; > + > if (pack_edges) > fclose(pack_edges); > - pack_edges = fopen(a + 20, "a"); > + pack_edges = fopen(filename, "a"); > if (!pack_edges) > - die("Cannot open %s: %s", a + 20, strerror(errno)); > + die("Cannot open %s: %s", filename, strerror(errno)); > } else if (!strcmp(a, "--force")) > force_update = 1; > else if (!strcmp(a, "--quiet")) Normally, the compiler takes care of such optimizations. It actually takes care of it much better than you can do yourself, and doing it yourself can even sometimes generate less optimized code because it gets in the compiler optimizations'way. Mike ``` ## Junio C Hamano, 2007-11-22 20:21 Subject: Re: [PATCH] Avoid recalculating filename string pointer. Message-ID: <7voddm3ubz.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7voddm3ubz.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20071122195457.GB19675@glandium.org> ``` Mike Hommey writes: > On Wed, Nov 21, 2007 at 10:59:41PM -0200, André Goddard Rosa wrote: >> --- a/fast-import.c >> +++ b/fast-import.c >> @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) >> else if (!prefixcmp(a, "--export-marks=")) >> mark_file = a + 15; >> else if (!prefixcmp(a, "--export-pack-edges=")) { >> + char *filename = a + 20; >> + >> if (pack_edges) >> fclose(pack_edges); >> - pack_edges = fopen(a + 20, "a"); >> + pack_edges = fopen(filename, "a"); >> if (!pack_edges) >> - die("Cannot open %s: %s", a + 20, strerror(errno)); >> + die("Cannot open %s: %s", filename, strerror(errno)); >> } else if (!strcmp(a, "--force")) >> force_update = 1; >> else if (!strcmp(a, "--quiet")) > > Normally, the compiler takes care of such optimizations. It actually > takes care of it much better than you can do yourself, and doing it > yourself can even sometimes generate less optimized code because it > gets in the compiler optimizations'way. True, but I think another point of the patch is to address the risk of two instances of "+ 20" going out of sync if/when the option parsing is updated. Not that I think André meant the patch as defensive coding (the subject suggests it was meant to be a micro-optimization), nor this is the good way to address that risk factor (parse-options may be a better match for it). ``` ## André Goddard Rosa, 2007-11-25 21:43 Subject: [Resend PATCH] Avoid recalculating filename string pointer. Message-ID: URL: https://gitlist.dev/e/b8bf37780711251343s7f10804fmd2558d38dc434eb4%40mail.gmail.com In-Reply-To: ``` Hi, all! Avoid calculating string position in 2 different places. From b6b05d9f8d8e053df4e971cd229e03b778c4d163 Mon Sep 17 00:00:00 2001 From: Andre Goddard Rosa Date: Tue, 27 Nov 2007 10:17:54 -0200 Subject: [PATCH] Avoid recalculating filename string pointer. Signed-off-by: Andre Goddard Rosa --- fast-import.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fast-import.c b/fast-import.c index 98c2bd5..2d262eb 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) else if (!prefixcmp(a, "--export-marks=")) mark_file = a + 15; else if (!prefixcmp(a, "--export-pack-edges=")) { + char *filename = a + 20; + if (pack_edges) fclose(pack_edges); - pack_edges = fopen(a + 20, "a"); + pack_edges = fopen(filename, "a"); if (!pack_edges) - die("Cannot open %s: %s", a + 20, strerror(errno)); + die("Cannot open %s: %s", filename, strerror(errno)); } else if (!strcmp(a, "--force")) force_update = 1; else if (!strcmp(a, "--quiet")) -- 1.5.3.6.861.gd794-dirty -- []s, André Goddard From b6b05d9f8d8e053df4e971cd229e03b778c4d163 Mon Sep 17 00:00:00 2001 From: Andre Goddard Rosa Date: Tue, 27 Nov 2007 10:17:54 -0200 Subject: [PATCH] Avoid recalculating filename string pointer. Signed-off-by: Andre Goddard Rosa --- fast-import.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fast-import.c b/fast-import.c index 98c2bd5..2d262eb 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2304,11 +2304,13 @@ int main(int argc, const char **argv) else if (!prefixcmp(a, "--export-marks=")) mark_file = a + 15; else if (!prefixcmp(a, "--export-pack-edges=")) { + char *filename = a + 20; + if (pack_edges) fclose(pack_edges); - pack_edges = fopen(a + 20, "a"); + pack_edges = fopen(filename, "a"); if (!pack_edges) - die("Cannot open %s: %s", a + 20, strerror(errno)); + die("Cannot open %s: %s", filename, strerror(errno)); } else if (!strcmp(a, "--force")) force_update = 1; else if (!strcmp(a, "--quiet")) -- 1.5.3.6.861.gd794-dirty ```