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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html