Re: [PATCH v2 3/3] Use die_errno() instead of die() when checking syscalls
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Jun 6, 2009, 21:02 UTC
- Message-ID
- <200906062302.08616.j6t@kdbg.org>
- In-Reply-To
- <62538974f2c0f4561428507e514daa87dbfcac01.1244299302.git.trast@student.ethz.ch>
On Samstag, 6. Juni 2009, Thomas Rast wrote:
Show 17 quoted lines
> 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> > ---
Show 9 quoted lines
> @@ -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.
Show 8 quoted lines
> @@ -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.
Show 8 quoted lines
> @@ -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.
Show 9 quoted lines
> @@ -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.
Show 11 quoted lines
> @@ -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.
Show 8 quoted lines
> @@ -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.
Show 11 quoted lines
> 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