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