Ralf Thielow wrote:
> In case of bad graft data which is determine on many
> places we go back to the first place of detection,
> so move it to the end of the function.
I like it. Three tiny nits.
Perhaps the log message could be clarified like so:
bad graft data is noticed in several places in read_graft_line and
in each case we go back to the first site of detection. Move the
error handling to the end of the function for better readability.
> --- a/commit.c
> +++ b/commit.c
> @@ -137,12 +137,8 @@ struct commit_graft *read_graft_line(char *buf, int len)
> buf[--len] = '\0';
> if (buf[0] == '#' || buf[0] == '\0')
> return NULL;
> - if ((len + 1) % 41) {
> - bad_graft_data:
> - error("bad graft data: %s", buf);
> - free(graft);
> - return NULL;
> - }
> + if ((len + 1) % 41)
> + goto bad_graft_data;
Trailing whitespace. (You can avoid this in the future with
"git diff --check".)
[...]
> @@ -155,6 +151,11 @@ struct commit_graft *read_graft_line(char *buf, int len)
> goto bad_graft_data;
> }
> return graft;
> +
> +bad_graft_data:
> + error("bad graft data: %s", buf);
A space before the "bad_graft_data:" label would improve future
diff --show-c-function output.
Except as noted above,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
---
diff --git a/commit.c b/commit.c
index d86159a..d5144f6 100644
--- a/commit.c
+++ b/commit.c
@@ -137,7 +137,7 @@ struct commit_graft *read_graft_line(char *buf, int len)
buf[--len] = '\0';
if (buf[0] == '#' || buf[0] == '\0')
return NULL;
- if ((len + 1) % 41)
+ if ((len + 1) % 41)
goto bad_graft_data;
i = (len + 1) / 41 - 1;
graft = xmalloc(sizeof(*graft) + 20 * i);
@@ -152,7 +152,7 @@ struct commit_graft *read_graft_line(char *buf, int len)
}
return graft;
-bad_graft_data:
+ bad_graft_data:
error("bad graft data: %s", buf);
free(graft);
return NULL;
--
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