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.

> 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;

and as a bonus, that keeps all of the logic together in one (fairly
readable) chain.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to