Re: [PATCH v3 4/4] stash: implement builtin stash

From: Jeff King <peff@peff.net>
Date: May 28, 2017, 19:26 UTC
Message-ID: <20170528192632.3iei3hxfggqkfrfh@sigill.intra.peff.net>
In-Reply-To: <CA+CzEk8Xx4nh=Bh533DR=Xo_BjQsxAoYNbx3=O=rtWr8+v4xsQ@mail.gmail.com>

On Sun, May 28, 2017 at 11:31:48AM -0700, Joel Teichroeb wrote:

Show 9 quoted lines

> >> +       /* TODO: Improve this logic */
> >> +       strbuf_addf(&symbolic, "%s", REV);
> >> +       str = strstr(symbolic.buf, "@");
> >
> > Could you elaborate on how this should be improved?
> 
> I just figured there would be a builtin function that could help here,
> but hadn't had the chance to look into it. It's something easy to do
> in bash, but more complicated in C.

There's no strbuf function for "truncate at this character". But:

  - you can use strchr for a single-character match, which is more
    efficient; i.e.:

      str = strchr(symbolic.buf, '@');

  - instead of inserting a '\0' into the strbuf, use strbuf_setlen(),
    which also updates the symbolic.len; i.e.:

      strbuf_setlen(&symbolic, str - symbolic.buf);

  - it looks like you copy into the strbuf just to truncate, so you
    could actually get the final size before inserting into the strbuf
    using strchrnul. Like:

      end_of_rev = strchrnul(REV, '@');
      strbuf_add(&symbolic, REV, end_of_rev - REV);

-Peff