On 01/14/2013 09:08 AM, Jim Meyering wrote:
Benno Schulenberg wrote:
On Fri, Jan 11, 2013, at 8:39, Jim Meyering wrote:
wwarn (_("%s: read failed"), src_name);
When things go wrong, I would prefer to see a word like
"failed", "error", "mistake", "bad", "invalid" or "mayday"
at the beginning of the line (right after the command name).
cmd: error in something: /some/complicated/filename
cmd: /some/complicated/filename: error in something
The first form is to me much clearer than the second.
That something went wrong is the main thing, with
which file exactly is secondary, in my opinion.
Good argument, as long as there isn't a line number.
Also, you might argue that the active-voiced
(I, the command) "failed to read"
is better than the passive-voiced
(a) "read failed"
If there are both file name and line number, then they should be
formatted like this, like compiler diagnostics:
CMD: FILE_NAME:LINE_NUMBER: diagnostic
so that diagnostic-parsing tools can handle them seamlessly.
There are many existing cases of "error {read,writ}ing" in coreutils.
So we may change all together, but that would be a further patch.
But I'll note that "error reading" and "error writing" are quite
unambiguous and fit well to indicate partial failure of the operation.
If we used "failed to write", then it might imply that nothing was written etc.
So I've adjusted Benno's patch as per the attached,
which rewraps long lines and s/error closing/failed to close/
as that is a single operation where "failed to" fits well.
I'll apply this in the next few hours.
thanks,
Pádraig.
diff --git a/src/copy.c b/src/copy.c
index 911ed81..3f2cc2b 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1172,13 +1172,13 @@ preserve_metadata:
close_src_and_dst_desc:
if (close (dest_desc) < 0)
{
- error (0, errno, _("error closing %s"), quote (dst_name));
+ error (0, errno, _("failed to close %s"), quote (dst_name));
return_val = false;
}
close_src_desc:
if (close (source_desc) < 0)
{
- error (0, errno, _("error closing %s"), quote (src_name));
+ error (0, errno, _("failed to close %s"), quote (src_name));
return_val = false;
}
diff --git a/src/cp.c b/src/cp.c
index 66bba85..e235b32 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1066,7 +1066,8 @@ main (int argc, char **argv)
{
struct stat st;
if (stat (optarg, &st) != 0)
- error (EXIT_FAILURE, errno, _("failed to access %s"), quote (optarg));
+ error (EXIT_FAILURE, errno, _("failed to access %s"),
+ quote (optarg));
if (! S_ISDIR (st.st_mode))
error (EXIT_FAILURE, 0, _("target %s is not a directory"),
quote (optarg));
diff --git a/src/dd.c b/src/dd.c
index 601b0e7..c98e578 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -2233,7 +2233,8 @@ main (int argc, char **argv)
|| fd_reopen (STDOUT_FILENO, output_file, O_RDWR | opts, perms) < 0)
&& (fd_reopen (STDOUT_FILENO, output_file, O_WRONLY | opts, perms)
< 0))
- error (EXIT_FAILURE, errno, _("failed to open %s"), quote (output_file));
+ error (EXIT_FAILURE, errno, _("failed to open %s"),
+ quote (output_file));
if (seek_records != 0 && !(conversions_mask & C_NOTRUNC))
{
diff --git a/src/head.c b/src/head.c
index 60b53f2..d79d5f7 100644
--- a/src/head.c
+++ b/src/head.c
@@ -859,7 +859,7 @@ head_file (const char *filename, uintmax_t n_units, bool count_lines,
ok = head (filename, fd, n_units, count_lines, elide_from_end);
if (!is_stdin && close (fd) != 0)
{
- error (0, errno, _("error closing %s"), quote (filename));
+ error (0, errno, _("failed to close %s"), quote (filename));
return false;
}
return ok;
diff --git a/src/install.c b/src/install.c
index e4c5c65..94374df 100644
--- a/src/install.c
+++ b/src/install.c
@@ -841,7 +841,8 @@ main (int argc, char **argv)
{
struct stat st;
if (stat (optarg, &st) != 0)
- error (EXIT_FAILURE, errno, _("failed to access %s"), quote (optarg));
+ error (EXIT_FAILURE, errno, _("failed to access %s"),
+ quote (optarg));
if (! S_ISDIR (st.st_mode))
error (EXIT_FAILURE, 0, _("target %s is not a directory"),
quote (optarg));
diff --git a/src/ln.c b/src/ln.c
index 5141388..1aa1473 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -512,7 +512,8 @@ main (int argc, char **argv)
{
struct stat st;
if (stat (optarg, &st) != 0)
- error (EXIT_FAILURE, errno, _("failed to access %s"), quote (optarg));
+ error (EXIT_FAILURE, errno, _("failed to access %s"),
+ quote (optarg));
if (! S_ISDIR (st.st_mode))
error (EXIT_FAILURE, 0, _("target %s is not a directory"),
quote (optarg));
diff --git a/src/mv.c b/src/mv.c
index 230502e..1cfcd82 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -397,7 +397,8 @@ main (int argc, char **argv)
{
struct stat st;
if (stat (optarg, &st) != 0)
- error (EXIT_FAILURE, errno, _("failed to access %s"), quote (optarg));
+ error (EXIT_FAILURE, errno, _("failed to access %s"),
+ quote (optarg));
if (! S_ISDIR (st.st_mode))
error (EXIT_FAILURE, 0, _("target %s is not a directory"),
quote (optarg));
diff --git a/src/touch.c b/src/touch.c
index 9c99c30..3a3ffbe 100644
--- a/src/touch.c
+++ b/src/touch.c
@@ -169,7 +169,7 @@ touch (const char *file)
{
if (close (STDIN_FILENO) != 0)
{
- error (0, errno, _("error closing %s"), quote (file));
+ error (0, errno, _("failed to close %s"), quote (file));
return false;
}
}
diff --git a/src/truncate.c b/src/truncate.c
index 2278a7a..8349cb6 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -414,7 +414,7 @@ main (int argc, char **argv)
errors |= !do_ftruncate (fd, fname, size, rsize, rel_mode);
if (close (fd) != 0)
{
- error (0, errno, _("error closing %s"), quote (fname));
+ error (0, errno, _("failed to close %s"), quote (fname));
errors = true;
}
}
