Re: [PATCH v2] format-patch --signature-file <file>

From: Junio C Hamano <gitster@pobox.com>
Date: May 19, 2014, 16:54 UTC
Message-ID: <xmqqd2f93e0m.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140517153943.GB31912@hudson.localdomain>

Jeremiah Mahler <jmmahler@gmail.com> writes:

Show 16 quoted lines

> On Sat, May 17, 2014 at 06:00:14AM -0400, Jeff King wrote:
>> 
>> If you wanted to know whether it was set, I guess you'd have to compare
>> it to the default, like:
>> 
>>   if (signature_file) {
>> 	if (signature && signature != git_version_string)
>> 		die("you cannot specify both a signature and a signature-file");
>> 	... read signature file ...
>>   }
>> 
>
> That works until someone changes the default value.
> But if they did that then some tests should fail.
>
> I like the address comparision which avoids a string comparision.

Well, "avoids" is not quite a correct phrasing, because !strcmp() would be wrong there. You cannot tell "the user did not set anything and the variable stayed as the default" and "the user explicitly gave us a string but it happened to be the same as the default" apart with !strcmp(). Address comparison is not just "avoids" but is the right thing to do in this case.

>> though it's a bit ugly that this code has to know what the default is.

Avoiding that is easy with an indirection, no? Something like this at the top:

  static const char *the_default_signature = git_version_string;
  static const char *signature = the_default_signature;

and comparing to see if signature points at the same address as the_default_signature would give you what you want, I think.