Björn Engelmann <BjEngelmann@xxxxxx> writes:
> @@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
> extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
> extern int ce_path_match(const struct cache_entry *ce, const char
> **pathspec);
> extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int
> write_object, const char *type);
> +extern int read_pipe(int fd, char** return_buf, unsigned long*
> return_size);
> extern int index_pipe(unsigned char *sha1, int fd, const char *type,
> int write_object);
I smell whitespace breakage around here...
> diff --git a/mktag.c b/mktag.c
> index 2328878..79c466c 100644
> --- a/mktag.c
> +++ b/mktag.c
>...
> @@ -114,21 +114,24 @@ int main(int argc, char **argv)
>
> setup_git_directory();
>
> - // Read the signature
> - size = 0;
> - for (;;) {
> - int ret = xread(0, buffer + size, MAXSIZE - size);
> - if (ret <= 0)
> - break;
> - size += ret;
> + if (read_pipe(0, &buffer, &size)) {
> + free(buffer);
> + die("could not read from stdin");
> }
> -
> +
Please do not introduce lines with trailing whitespaces.
> // Verify it for some basic sanity: it needs to start with "object
> <sha1>\ntype\ntagger "
> - if (verify_tag(buffer, size) < 0)
> + if (verify_tag(buffer, size) < 0) {
> + free(buffer);
> die("invalid tag signature file");
> + }
You seem to be striving for extra cleanliness, but I personally
consider it is not worth calling free() immediately before you
call die().
>
> - if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
> + if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
> + free(buffer);
> die("unable to write tag file");
> + }
> +
> + free(buffer);
> +
> printf("%s\n", sha1_to_hex(result_sha1));
> return 0;
> }
A call to free() immediately before returning from main() might
look similar to the die() issue I mentioned above, but we might
extend it to do a lot more after writing the tag in the future,
so this one is very good to keep.
-
: 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