Re: [PATCH 66/67] use strbuf_complete to conditionally append slash

From: Jeff King <peff@peff.net>
Date: Sep 16, 2015, 22:39 UTC
Message-ID: <20150916223901.GA24945@sigill.intra.peff.net>
In-Reply-To: <xmqq8u86m2i4.fsf@gitster.mtv.corp.google.com>

On Wed, Sep 16, 2015 at 03:18:59PM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > diff --git a/imap-send.c b/imap-send.c
> > index 01aa227..f5d2b06 100644
> > --- a/imap-send.c
> > +++ b/imap-send.c
> > @@ -1412,8 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
> >  	curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
> >  
> >  	strbuf_addstr(&path, server.host);
> > -	if (!path.len || path.buf[path.len - 1] != '/')
> > -		strbuf_addch(&path, '/');
> > +	strbuf_complete(&path, '/');
> >  	strbuf_addstr(&path, server.folder);
> 
> Is this conversion correct?  This seems to me that the caller wants
> to create an IMAP folder name immediately under the root hierarchy
> and wants to have the leading slash in the result.

Ugh, you're right. This is the "other" style Eric mentioned earlier.

This looks like the only one in the patch (there are many that did not check buf.len at all, but if we assume they were not invoking undefined behavior before, then they are fine under the new code).

-Peff