From: Joshua Juran Date: Tue, 03 May 2011 08:17:17 GMT Subject: Re: [Annoyance] "git log .." thinks ".." is ambiguous Message-ID: <86404CD9-6040-4083-912F-23F4C11F4ED2@gmail.com> In-Reply-To: <4DBFA1A2.6050409@drmicha.warpmail.net> On May 2, 2011, at 11:33 PM, Michael J Gruber wrote: > Joshua Juran venit, vidit, dixit 03.05.2011 03:57: >> On May 2, 2011, at 12:15 PM, Junio C Hamano wrote: >> >>> diff --git a/revision.c b/revision.c >>> index f4b8b38..26271d1 100644 >>> --- a/revision.c >>> +++ b/revision.c >>> @@ -905,14 +905,26 @@ int handle_revision_arg(const char *arg, >>> struct rev_info *revs, >>> const char *this = arg; >>> int symmetric = *next == '.'; >>> unsigned int flags_exclude = flags ^ UNINTERESTING; >>> + static const char head_by_default[] = "HEAD"; >>> >>> *dotdot = 0; >>> next += symmetric; >>> >>> if (!*next) >>> - next = "HEAD"; >>> + next = head_by_default; >>> if (dotdot == arg) >>> - this = "HEAD"; >>> + this = head_by_default; >>> + if (this == head_by_default && next == head_by_default && >>> + !symmetric) { >> >> Is there a reason not to write >> >> const char *head_by_default = "HEAD"; >> >> or even >> >> const char *const head_by_default = "HEAD"; >> >> instead? Loading a static array and checking an init flag is a >> pessimization versus just pointing into a read-only segment. > > Because of the comparisons later on: this == "HEAD" is not the same. Sorry if I was unclear. I meant to replace only the 'static const char head_by_default[] = "HEAD";' line and leave the rest of the patch unchanged. My recollection is that Metrowerks C will statically allocate read- write storage (duplicating the read-only copy of the string) and copy the string into it at runtime. It only copies the string once, which is ensured by checking an internally generated flag (also statically allocated) every time the scope containing the declaration is executed. This is the pessimization I speak of. By contrast, my suggestion allocates a single pointer on the stack regardless of compiler optimization. Josh