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

[Jonathan Nieder <jrnieder@xxxxxxxxx>]

David Barr wrote:

> 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)
[...]
> @@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html