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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 17, 2014 at 06:00:14AM -0400, Jeff King wrote:
> On Sat, May 17, 2014 at 01:59:11AM -0700, Jeremiah Mahler wrote:
> 
> > >   if (signature) {
> > > 	if (signature_file)
> > > 		die("you cannot specify both a signature and a signature-file");
> > > 	/* otherwise, we already have the value */
> > >   } else if (signature_file) {
> > > 	struct strbuf buf = STRBUF_INIT;
> > > 	strbuf_read(&buf, signature_file, 128);
> > > 	signature = strbuf_detach(&buf);
> > >   } else
> > > 	signature = git_version_string;
> > > 
> > 
> > Before, --no-signature would clear the &signature.
> > With this code it sees it as not being set and assigns
> > the default version string.
> 
> Ah, you're right. Thanks for catching it.
> 
> 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.

> though it's a bit ugly that this code has to know what the default is.
> Having signature-file take precedence is OK with me, but it feels
> somewhat arbitrary to me from the user's perspective.
> 
> -Peff

-- 
Jeremiah Mahler
jmmahler@xxxxxxxxx
http://github.com/jmahler
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]