On 05/10/2014 08:39 PM, Jim Meyering wrote: > On Sat, May 10, 2014 at 11:42 AM, Paul Eggert <[email protected]> wrote: >> > * src/shred.c (main): Limit -n (number of passes) value to >> > ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long. >> > Limit the -s (file size) value to OFF_T_MAX. >> > --- >> > src/shred.c | 9 +++++---- >> > 1 file changed, 5 insertions(+), 4 deletions(-) >> > >> > diff --git a/src/shred.c b/src/shred.c >> > index 607c6be..f4347e0 100644 >> > --- a/src/shred.c >> > +++ b/src/shred.c > ... >> > @@ -1256,9 +1256,10 @@ main (int argc, char **argv) >> > >> > case 's': >> > { >> > - uintmax_t tmp; >> > - if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") >> > - != LONGINT_OK) >> > + intmax_t tmp; >> > + if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") >> > + != LONGINT_OK) >> > + || OFF_T_MAX < tmp) > Hi Paul, > The above makes it so shred now accepts a negative size. > Before, that would be diagnosed as invalid: > > $ shred -s-1 k > shred: -1: invalid file size > > With a size of -2, shred will write 64KB blocks forever -- or until it > runs out of space. > > Here's a patch to fix that and to add a test covering that case: > > > 0001-shred-don-t-infloop-upon-negative-size.patch > > > From d7cfcbef7eb2cd12ac83e5c1c123de2015adbef9 Mon Sep 17 00:00:00 2001 > From: Jim Meyering <[email protected]> > Date: Sat, 10 May 2014 12:36:16 -0700 > Subject: [PATCH] shred: don't infloop upon negative size > > * src/shred.c (main): With the preceding change, shred -s-2 FILE > would write 64KB blocks forever -- or until disk full. This change > makes shred reject a negative size. > * tests/misc/shred-negative.sh: New file. > * tests/local.mk (all_tests): Add it. > --- > src/shred.c | 4 ++-- > tests/local.mk | 1 + > tests/misc/shred-negative.sh | 28 ++++++++++++++++++++++++++++ > 3 files changed, 31 insertions(+), 2 deletions(-) > create mode 100755 tests/misc/shred-negative.sh > > diff --git a/src/shred.c b/src/shred.c > index f4347e0..bd88e38 100644 > --- a/src/shred.c > +++ b/src/shred.c > @@ -1256,8 +1256,8 @@ main (int argc, char **argv) > > case 's': > { > - intmax_t tmp; > - if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > + uintmax_t tmp; > + if ((xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > != LONGINT_OK) > || OFF_T_MAX < tmp) > { > diff --git a/tests/local.mk b/tests/local.mk > index 5286bfb..cd7da5b 100644 > --- a/tests/local.mk > +++ b/tests/local.mk > @@ -313,6 +313,7 @@ all_tests = \ > tests/misc/sha384sum.pl \ > tests/misc/sha512sum.pl \ > tests/misc/shred-exact.sh \ > + tests/misc/shred-negative.sh \ > tests/misc/shred-passes.sh \ > tests/misc/shred-remove.sh \ > tests/misc/shuf.sh \ > diff --git a/tests/misc/shred-negative.sh b/tests/misc/shred-negative.sh > new file mode 100755 > index 0000000..86cbf3e > --- /dev/null > +++ b/tests/misc/shred-negative.sh > @@ -0,0 +1,28 @@ > +#!/bin/sh > +# Exercise shred -s-3 FILE > + > +# Copyright (C) 2014 Free Software Foundation, Inc. > + > +# This program is free software: you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation, either version 3 of the License, or > +# (at your option) any later version. > + > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > + > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src > +print_ver_ shred > + > +echo 'shred: -2: invalid file size' > exp || framework_failure_ > +echo 1234 > f || framework_failure_ > + > +shred -s-2 f 2>err && fail=1 > +compare exp err || fail=1 > + > +Exit $fail > -- 2.0.0.rc0.38.g1697bf3
Ah great you beat me to it :) Code looks good, as does the test. Please push. I've marked this bug as done. thanks! Pádraig.
