Re: [PATCH 7/7] vcs-svn: fix clang-analyzer warning

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 24, 2012, 14:33 UTC
Message-ID: <20120524143337.GB3732@burratino>
In-Reply-To: <1337868259-45626-8-git-send-email-davidbarr@google.com>

David Barr wrote:

Show 7 quoted lines

> vcs-svn/svndiff.c:278:3: warning: expression result unused [-Wunused-value]
>                 error("invalid delta: incorrect postimage length");
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from vcs-svn/svndiff.c:6:
> vcs-svn/compat-util.h:18:61: note: instantiated from:
> #define error(...) (fprintf(stderr, "error: " __VA_ARGS__), -1)
>                                                             ^~

Yuck.  Would you be ok with an inline variadic function?

 static inline int error(const char *fmt, ...)
 {
	va_list ap;

	fprintf(stderr, "error: ");

	va_start(ap, fmt);
	vfprintf(stderr, fmt, ap)
	va_end(ap);

	fprintf(stderr, "\n");

	return -1;
 }

The error() macro above also seems to leave out a newline.

> --- a/vcs-svn/svndiff.c
> +++ b/vcs-svn/svndiff.c
> @@ -258,6 +258,7 @@ static int apply_window_in_core(struct window *ctx)

[...]

Show 17 quoted lines

> @@ -275,16 +276,15 @@ static int apply_one_window(struct line_buffer *delta, off_t *delta_len,
>  	if (apply_window_in_core(&ctx))
>  		goto error_out;
>  	if (ctx.out.len != out_len) {
> -		error("invalid delta: incorrect postimage length");
> +		rv = error("invalid delta: incorrect postimage length");
>  		goto error_out;
>  	}
>  	if (write_strbuf(&ctx.out, out))
>  		goto error_out;
> -	window_release(&ctx);
> -	return 0;
> +	rv = 0;
>  error_out:
>  	window_release(&ctx);
> -	return -1;
> +	return rv;

That said, if this change is justified by saying that it avoids having to repeat the cleanup code, it already looks like a good change. The commit message could mention that the original motivation and a side-benefit is to help the standalone version that has a slightly crazier definition of error().

Jonathan