[PATCH] dd doesn't return a failure status
Kazuo TAKADA
kztakada at sm.sony.co.jp
Wed Oct 3 09:38:24 UTC 2007
loic.grenie at gmail.com wrote:
{snip..}
>
> It may be faster to do
>
>if ((w = write_and_stats(...)))
> goto out_status;
>
> and
>
>return w;
>
> because write_and_stats returns 1 on failure. You can even tweak
> it to return either EXIT_SUCCESS or EXIT_FAILURE and change the
> if to:
>
>if ((w = write_and_stats(...)) == EXIT_SUCCESS)
> goto out_status;
>
> (which is the same but symbolically different).
I agree with your suggestion basically and I had already tried.
But, in other lines, the variable 'w' is set with the written size by
the function 'full_write_or_warn()'.
An exit status of zero must indicate success, and a nonzero value
indicates failure, therefore, 'return w' is not appropriate.
Well, if I take care of variable's meanings, it can be written as below.
----------------------------------------------------------------------
--- coreutils/dd.c.orig 2007-09-03 20:48:39.000000000 +0900
+++ coreutils/dd.c 2007-10-03 18:19:29.000000000 +0900
@@ -54,17 +54,17 @@
return n;
}
-static bool write_and_stats(int fd, const void *buf, size_t len, size_t obs,
+static int write_and_stats(int fd, const void *buf, size_t len, size_t obs,
const char *filename)
{
ssize_t n = full_write_or_warn(fd, buf, len, filename);
if (n < 0)
- return 1;
+ return EXIT_FAILURE;
if (n == obs)
G.out_full++;
else if (n) /* > 0 */
G.out_part++;
- return 0;
+ return EXIT_SUCCESS;
}
#if ENABLE_LFS
@@ -106,7 +106,8 @@
#endif
};
size_t ibs = 512, obs = 512;
- ssize_t n, w;
+ ssize_t n;
+ int retval = EXIT_SUCCESS;
char *ibuf, *obuf;
/* And these are all zeroed at once! */
struct {
@@ -303,18 +304,21 @@
tmp += d;
oc += d;
if (oc == obs) {
- if (write_and_stats(ofd, obuf, obs, obs, outfile))
+ if ((retval = write_and_stats(ofd, obuf, obs, obs, outfile)))
goto out_status;
oc = 0;
}
}
- } else if (write_and_stats(ofd, ibuf, n, obs, outfile))
+ } else if ((retval = write_and_stats(ofd, ibuf, n, obs, outfile)))
goto out_status;
}
if (ENABLE_FEATURE_DD_IBS_OBS && oc) {
- w = full_write_or_warn(ofd, obuf, oc, outfile);
- if (w < 0) goto out_status;
+ ssize_t w = full_write_or_warn(ofd, obuf, oc, outfile);
+ if (w < 0) {
+ retval = EXIT_FAILURE;
+ goto out_status;
+ }
if (w > 0)
G.out_part++;
}
@@ -330,5 +334,5 @@
out_status:
dd_output_status(0);
- return EXIT_SUCCESS;
+ return retval;
}
----------------------------------------------------------------------
It may be clear on the point of the code's policy.
Thanks.
Kazuo TAKADA
More information about the busybox
mailing list