| Message ID | 20190919214712.7348-4-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | name-rev: eliminate recursion | expand |
Am 19.09.19 um 23:46 schrieb SZEDER Gábor: > Use strip_suffix() instead of open-coding it, making the code more > idiomatic. > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > --- > builtin/name-rev.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/builtin/name-rev.c b/builtin/name-rev.c > index c785fe16ba..d345456656 100644 > --- a/builtin/name-rev.c > +++ b/builtin/name-rev.c > @@ -317,11 +317,11 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf) > if (!n->generation) > return n->tip_name; > else { > - int len = strlen(n->tip_name); > - if (len > 2 && !strcmp(n->tip_name + len - 2, "^0")) > - len -= 2; > + size_t len; > + strip_suffix(n->tip_name, "^0", &len); > strbuf_reset(buf); > - strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation); > + strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name, > + n->generation); > return buf->buf; > } > } > This gets rid of the repeated magic string length constant 2, which is nice. But why not go all the way to full strbuf-ness? It's shorter, looks less busy, and the extra two copied bytes shouldn't matter in a measurable way. else { strbuf_reset(buf); strbuf_addstr(buf, n->tip_name); strbuf_strip_suffix(buf, "^0"); strbuf_addf(buf, "~%d", n->generation); return buf->buf; }
On Fri, Sep 20, 2019 at 06:36:30PM +0200, René Scharfe wrote: > Am 19.09.19 um 23:46 schrieb SZEDER Gábor: > > Use strip_suffix() instead of open-coding it, making the code more > > idiomatic. > > > > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> > > --- > > builtin/name-rev.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/builtin/name-rev.c b/builtin/name-rev.c > > index c785fe16ba..d345456656 100644 > > --- a/builtin/name-rev.c > > +++ b/builtin/name-rev.c > > @@ -317,11 +317,11 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf) > > if (!n->generation) > > return n->tip_name; > > else { > > - int len = strlen(n->tip_name); > > - if (len > 2 && !strcmp(n->tip_name + len - 2, "^0")) > > - len -= 2; > > + size_t len; > > + strip_suffix(n->tip_name, "^0", &len); > > strbuf_reset(buf); > > - strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation); > > + strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name, > > + n->generation); > > return buf->buf; > > } > > } > > > > This gets rid of the repeated magic string length constant 2, which is > nice. But why not go all the way to full strbuf-ness? It's shorter, > looks less busy, and the extra two copied bytes shouldn't matter in a > measurable way. > > else { > strbuf_reset(buf); > strbuf_addstr(buf, n->tip_name); > strbuf_strip_suffix(buf, "^0"); > strbuf_addf(buf, "~%d", n->generation); > return buf->buf; > } Oh, I like this, thanks!
diff --git a/builtin/name-rev.c b/builtin/name-rev.c index c785fe16ba..d345456656 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -317,11 +317,11 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf) if (!n->generation) return n->tip_name; else { - int len = strlen(n->tip_name); - if (len > 2 && !strcmp(n->tip_name + len - 2, "^0")) - len -= 2; + size_t len; + strip_suffix(n->tip_name, "^0", &len); strbuf_reset(buf); - strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation); + strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name, + n->generation); return buf->buf; } }
Use strip_suffix() instead of open-coding it, making the code more idiomatic. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- builtin/name-rev.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)