On Sat, May 17, 2014 at 03:42:24AM -0400, Jeff King wrote:
> 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.
> > 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@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