Re: [PATCH v2] format-patch --signature-file <file>
- From
Jeremiah Mahler <jmmahler@gmail.com>
- Date
- May 17, 2014, 08:59 UTC
- Message-ID
- <20140517085911.GA18862@hudson.localdomain>
- In-Reply-To
- <20140517074224.GA16697@sigill.intra.peff.net>
On Sat, May 17, 2014 at 03:42:24AM -0400, Jeff King wrote:
Show 19 quoted lines
> On Sat, May 17, 2014 at 12:25:48AM -0700, Jeremiah Mahler wrote: > > > > We have routines for reading directly into a strbuf, which eliminates > > > the need for this 1024-byte limit. We even have a wrapper that can make > > > this much shorter: > > > > > > struct strbuf buf = STRBUF_INIT; > > > > > > strbuf_read_file(&buf, arg, 128); > > > *signature = strbuf_detach(&buf, NULL); > > > > > > > Yes, that is much cleaner. > > The memory returned by strbuf_detach() will have to be freed as well. > > In cases like this, we often let the memory leak. It's in a global that > stays valid through the whole program, so we just let the program's exit > clean it up. >
It bugs me but I see your point. It works just fine in this situation.
Show 23 quoted lines
> > Having --signature-file override --signature seems simpler to implement.
> > The signature variable has a default value which complicates
> > determining whether it was set or not.
>
> Yeah, the default value complicates it. I think you can handle that just
> by moving the default to the main logic, like:
>
> static const char *signature;
> static const char *signature_file;
>
> ...
>
> 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.
> and as a bonus, that keeps all of the logic together in one (fairly > readable) chain. > > -Peff
-- Jeremiah Mahler jmmahler@gmail.com http://github.com/jmahler