From: Junio C Hamano <junio@pobox.com>
Date: Thu, 26 Apr 2012 23:25:41 GMT
Subject: Re: [PATCH v2 08/18] do_for_each_reflog(): use a strbuf to hold logfile name
Message-ID: <xmqqaa1y6poq.fsf@junio.mtv.corp.google.com>
In-Reply-To: <1335479227-7877-9-git-send-email-mhagger@alum.mit.edu>

mhagger@alum.mit.edu writes:

> From: Michael Haggerty <mhagger@alum.mit.edu>
>
> This simplifies the bookkeeping and allows an (artificial) restriction
> on refname component length to be removed.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs.c |   45 +++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 1d25151..f43c255 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2248,44 +2248,45 @@ static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
>  	int retval = 0;
>  	struct dirent *de;
>  	int baselen;
> -	char *log;
> +	struct strbuf log;
>  
>  	if (!d)
>  		return *base ? errno : 0;
>  
>  	baselen = strlen(base);
> -	log = xmalloc(baselen + 257);
> -	memcpy(log, base, baselen);
> -	if (baselen && base[baselen-1] != '/')
> -		log[baselen++] = '/';
> +	strbuf_init(&log, baselen + 257);
> +	strbuf_add(&log, base, baselen);
> +	if (log.len && log.buf[log.len-1] != '/') {
> +		strbuf_addch(&log, '/');
> +		baselen++;
> +	}
>  
>  	while ((de = readdir(d)) != NULL) {
>  		struct stat st;
> -		int namelen;
>  
>  		if (de->d_name[0] == '.')
>  			continue;
> -		namelen = strlen(de->d_name);
> -		if (namelen > 255)
> -			continue;
>  		if (has_extension(de->d_name, ".lock"))
>  			continue;
> -		memcpy(log + baselen, de->d_name, namelen+1);
> -		if (stat(git_path("logs/%s", log), &st) < 0)
> -			continue;
> -		if (S_ISDIR(st.st_mode)) {
> -			retval = do_for_each_reflog(log, fn, cb_data);
> +		strbuf_addstr(&log, de->d_name);
> +		if (stat(git_path("logs/%s", log.buf), &st) < 0) {
> +			/* Silently ignore. */
>  		} else {

Please write this like this:

		if (...) {
			; /* silently ignore */
		}

to make the "emptyness" stand out (I amended the previous round when I
queued them to 'pu', but I forgot to point it out in my review message).