Re: [PATCH v2 3/3] Use die_errno() instead of die() when checking syscalls

[Johannes Sixt <j6t@kdbg.org>]

On Samstag, 6. Juni 2009, Thomas Rast wrote:
> Lots of die() calls did not actually report the kind of error, which
> can leave the user confused as to the real problem.  Use die_errno()
> where we check a system/library call that sets errno on failure, or
> one of the following that wrap such calls:
>
>   Function              Passes on error from
>   --------              --------------------
>   odb_pack_keep         open
>   read_ancestry         fopen
>   read_in_full          xread
>   strbuf_read           xread
>   strbuf_read_file      open or strbuf_read_file
>   strbuf_readlink       readlink
>   write_in_full         xwrite
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---

> @@ -2262,7 +2262,6 @@ int cmd_blame(int argc, const char **argv, const char
> *prefix)
>
>  	if (revs_file && read_ancestry(revs_file))
>  		die_errno("reading graft file '%s' failed", revs_file);
> -
>  	if (cmd_is_annotate) {
>  		output_option |= OUTPUT_ANNOTATE_COMPAT;
>  		blame_date_mode = DATE_ISO8601;

Unrelated and not an improvement.

> @@ -220,13 +220,12 @@ static void copy_or_link_directory(struct strbuf
> *src, struct strbuf *dest)
>
>  	dir = opendir(src->buf);
>  	if (!dir)
> -		die("failed to open %s", src->buf);
> -
> +		die_errno("failed to open '%s'", src->buf);

Here (and in other cases) you remote an empty line. I don't think that is an 
improvement.

> @@ -472,7 +472,6 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix) fp = fopen(git_path(commit_editmsg), "w");
>  	if (fp == NULL)
>  		die_errno("could not open '%s'", git_path(commit_editmsg));
> -
>  	if (cleanup_mode != CLEANUP_NONE)
>  		stripspace(&sb, 0);
>

Unrelated.

> @@ -496,7 +495,6 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix)
>
>  	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
>  		die_errno("could not write commit template");
> -
>  	strbuf_release(&sb);
>
>  	determine_author_info();

Ditto.

> @@ -1018,8 +1017,10 @@ int cmd_commit(int argc, const char **argv, const
> char *prefix)
>
>  	if (commit_index_files())
>  		die ("Repository has been updated, but unable to write\n"
> -		     "new_index file. Check that disk is not full or quota is\n"
> -		     "not exceeded, and then \"git reset HEAD\" to recover.");
> +		     "new_index file: %s.\n"
> +		     "Check that disk is not full or quota is not exceeded,\n"
> +		     "and then \"git reset HEAD\" to recover.",
> +		     strerror(errno));

This change should probably not be in this patch.

> @@ -452,7 +452,6 @@ static void import_marks(char *input_file)
>  	FILE *f = fopen(input_file, "r");
>  	if (!f)
>  		die_errno("cannot read '%s'", input_file);
> -
>  	while (fgets(line, sizeof(line), f)) {
>  		uint32_t mark;
>  		char *line_end, *mark_end;

Unrelated.

> diff --git a/csum-file.c b/csum-file.c
> index 9cc93ba..4d50cc5 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -55,8 +55,7 @@ int sha1close(struct sha1file *f, unsigned char *result,
> unsigned int flags) if (flags & CSUM_FSYNC)
>  			fsync_or_die(f->fd, f->name);
>  		if (close(f->fd))
> -			die_errno("%s: sha1 file error on close",
> -			    f->name);
> +			die_errno("%s: sha1 file error on close", f->name);

This should be in 2/3.

-- Hannes