From e2532ab5f2446ec736b10b24f57a36456deb197f Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Sun, 2 Dec 2007 01:44:42 +0000 Subject: [PATCH] dd: fix a bug where we don't report write errors testsuite: small cleanup full_write_or_warn 38 40 +2 write_and_stats 66 67 +1 dd_main 1358 1335 -23 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/1 up/down: 3/-23) Total: -20 bytes --- coreutils/dd.c | 41 +++++++++++---------- testsuite/runtest | 92 +++++++++++++++++++++-------------------------- 2 files changed, 63 insertions(+), 70 deletions(-) diff --git a/coreutils/dd.c b/coreutils/dd.c index fd4e7e8a2..7552c8518 100644 --- a/coreutils/dd.c +++ b/coreutils/dd.c @@ -14,6 +14,11 @@ /* This is a NOEXEC applet. Be very careful! */ +enum { + ifd = STDIN_FILENO, + ofd = STDOUT_FILENO, +}; + static const struct suffix_mult dd_suffixes[] = { { "c", 1 }, { "w", 2 }, @@ -45,19 +50,19 @@ static void dd_output_status(int ATTRIBUTE_UNUSED cur_signal) G.out_full, G.out_part); } -static ssize_t full_write_or_warn(int fd, const void *buf, size_t len, +static ssize_t full_write_or_warn(const void *buf, size_t len, const char *const filename) { - ssize_t n = full_write(fd, buf, len); + ssize_t n = full_write(ofd, buf, len); if (n < 0) bb_perror_msg("writing '%s'", filename); return n; } -static bool write_and_stats(int fd, const void *buf, size_t len, size_t obs, +static bool write_and_stats(const void *buf, size_t len, size_t obs, const char *filename) { - ssize_t n = full_write_or_warn(fd, buf, len, filename); + ssize_t n = full_write_or_warn(buf, len, filename); if (n < 0) return 1; if (n == obs) @@ -105,13 +110,13 @@ int dd_main(int argc, char **argv) OP_conv_noerror, #endif }; + int exitcode = EXIT_FAILURE; size_t ibs = 512, obs = 512; ssize_t n, w; char *ibuf, *obuf; /* And these are all zeroed at once! */ struct { int flags; - int ifd, ofd; size_t oc; off_t count; off_t seek, skip; @@ -121,8 +126,6 @@ int dd_main(int argc, char **argv) #endif } Z; #define flags (Z.flags ) -#define ifd (Z.ifd ) -#define ofd (Z.ofd ) #define oc (Z.oc ) #define count (Z.count ) #define seek (Z.seek ) @@ -133,6 +136,7 @@ int dd_main(int argc, char **argv) memset(&Z, 0, sizeof(Z)); INIT_G(); + //fflush(NULL); - is this needed because of NOEXEC? #if ENABLE_FEATURE_DD_SIGNAL_HANDLING sigact.sa_handler = dd_output_status; @@ -227,9 +231,8 @@ int dd_main(int argc, char **argv) obuf = xmalloc(obs); } if (infile != NULL) - ifd = xopen(infile, O_RDONLY); + xmove_fd(xopen(infile, O_RDONLY), ifd); else { - /* ifd = STDIN_FILENO; - it's zero and it's already there */ infile = bb_msg_standard_input; } if (outfile != NULL) { @@ -238,7 +241,7 @@ int dd_main(int argc, char **argv) if (!seek && !(flags & FLAG_NOTRUNC)) oflag |= O_TRUNC; - ofd = xopen(outfile, oflag); + xmove_fd(xopen(outfile, oflag), ofd); if (seek && !(flags & FLAG_NOTRUNC)) { if (ftruncate(ofd, seek * obs) < 0) { @@ -250,7 +253,6 @@ int dd_main(int argc, char **argv) } } } else { - ofd = STDOUT_FILENO; outfile = bb_msg_standard_output; } if (skip) { @@ -276,11 +278,10 @@ int dd_main(int argc, char **argv) if (n == 0) break; if (n < 0) { - if (flags & FLAG_NOERROR) { - n = ibs; - bb_simple_perror_msg(infile); - } else + if (!(flags & FLAG_NOERROR)) goto die_infile; + n = ibs; + bb_simple_perror_msg(infile); } if ((size_t)n == ibs) G.in_full++; @@ -303,17 +304,17 @@ int dd_main(int argc, char **argv) tmp += d; oc += d; if (oc == obs) { - if (write_and_stats(ofd, obuf, obs, obs, outfile)) + if (write_and_stats(obuf, obs, obs, outfile)) goto out_status; oc = 0; } } - } else if (write_and_stats(ofd, ibuf, n, obs, outfile)) + } else if (write_and_stats(ibuf, n, obs, outfile)) goto out_status; } if (ENABLE_FEATURE_DD_IBS_OBS && oc) { - w = full_write_or_warn(ofd, obuf, oc, outfile); + w = full_write_or_warn(obuf, oc, outfile); if (w < 0) goto out_status; if (w > 0) G.out_part++; @@ -327,8 +328,10 @@ int dd_main(int argc, char **argv) die_outfile: bb_simple_perror_msg_and_die(outfile); } + + exitcode = EXIT_SUCCESS; out_status: dd_output_status(0); - return EXIT_SUCCESS; + return exitcode; } diff --git a/testsuite/runtest b/testsuite/runtest index 92cbfdf4e..93d5ed6e1 100755 --- a/testsuite/runtest +++ b/testsuite/runtest @@ -1,20 +1,20 @@ #!/bin/sh -# Run old-style test. - +# Run one old-style test. +# Tests are stored in applet/testcase shell scripts. +# They are run using "sh -x -e applet/testcase". +# Option -e will make testcase stop on the first failed command. run_applet_testcase() { local applet=$1 local testcase=$2 - local status=0 - local RES= - + local status local uc_applet=$(echo $applet | tr a-z A-Z) local testname=$(basename $testcase) if grep -q "^# CONFIG_${uc_applet} is not set$" $bindir/.config; then - echo UNTESTED: $testname + echo "UNTESTED: $testname" return 0 fi @@ -22,72 +22,67 @@ run_applet_testcase() local feature=`sed -ne 's/^# FEATURE: //p' $testcase` if grep -q "^# ${feature} is not set$" $bindir/.config; then - echo UNTESTED: $testname + echo "UNTESTED: $testname" return 0 fi fi - rm -rf tmp - mkdir -p tmp - pushd tmp > /dev/null - -# echo Running testcase $testcase - d=$tsdir sh -x -e $testcase >.logfile.txt 2>&1 || status=$? - - if [ $status -ne 0 ]; then - echo FAIL: $testname - if [ $verbose -gt 0 ]; then - cat .logfile.txt + rm -rf ".tmpdir.$applet" + mkdir -p ".tmpdir.$applet" + cd ".tmpdir.$applet" || return 1 + +# echo "Running testcase $testcase" + d="$tsdir" sh -x -e "$testcase" >"$testname.stdout.txt" 2>&1 + status=$? + if [ $status != 0 ]; then + echo "FAIL: $testname" + if [ x"$VERBOSE" != x ]; then + cat "$testname.stdout.txt" fi - status=$? else - echo PASS: $testname - rm -f .logfile.txt - status=$? + echo "PASS: $testname" fi - popd > /dev/null - rm -rf tmp + cd .. + rm -rf ".tmpdir.$applet" return $status } +# Run all old-style tests for given applet run_applet_tests() { local applet=$1 - local status=0 - for testcase in $tsdir/$applet/*; do if [ "$testcase" = "$tsdir/$applet/CVS" ]; then continue fi - if ! run_applet_testcase $applet $testcase; then - status=1 - fi + run_applet_testcase $applet $testcase + test $? = 0 || status=1 done - return $status } -status=0 -verbose=0 [ -n "$tsdir" ] || tsdir=$(pwd) [ -n "$bindir" ] || bindir=$(dirname $(pwd)) PATH="$bindir:$PATH" +if [ x"$VERBOSE" = x ]; then + export VERBOSE= +fi + if [ x"$1" = x"-v" ]; then - verbose=1 - export VERBOSE=$verbose + export VERBOSE=1 shift fi implemented=$( $bindir/busybox 2>&1 | while read line; do - if test x"$line" = x"Currently defined functions:"; then + if [ x"$line" = x"Currently defined functions:" ]; then xargs | sed 's/,//g' break fi @@ -109,39 +104,34 @@ for i in $implemented; do done # Set up option flags so tests can be selective. +export OPTIONFLAGS=:$(sed -nr 's/^CONFIG_//p' $bindir/.config | sed 's/=.*//' | xargs | sed 's/ /:/g') -configfile=${bindir}/.config -export OPTIONFLAGS=:$(sed -nr 's/^CONFIG_(.*)=.*/\1/p' $configfile | xargs | sed 's/ /:/g') - +status=0 for applet in $applets; do if [ "$applet" = "links" ]; then continue; fi + + # Any old-style tests for this applet? if [ "$applet" != "CVS" -a -d "$tsdir/$applet" ]; then - if ! run_applet_tests $applet; then - status=1 - fi + run_applet_tests "$applet" + test $? = 0 || status=1 fi # Is this a new-style test? - if [ -f ${applet}.tests ]; then + if [ -f "${applet}.tests" ]; then if [ ! -h "$LINKSDIR/$applet" ] && [ "${applet:0:4}" != "all_" ]; then echo "SKIPPED: $applet (not built)" continue fi - if PATH="$LINKSDIR:$tsdir:$bindir:$PATH" \ - "${tsdir:-.}/$applet".tests - then - : - else - status=1 - fi +# echo "Running test ${tsdir:-.}/${applet}.tests" + PATH="$LINKSDIR:$tsdir:$bindir:$PATH" "${tsdir:-.}/${applet}.tests" + test $? = 0 || status=1 fi - done # Leaving the dir makes it somewhat easier to run failed test by hand #rm -rf "$LINKSDIR" if [ $status != 0 -a x"$VERBOSE" = x ]; then - echo "Failures detected, running with VERBOSE=1 will give more info" + echo "Failures detected, running with -v (verbose) will give more info" fi exit $status -- 2.25.1