On 01/22/2013 02:53 PM, Ondrej Oprala wrote: > The patch is based on this entry > https://bugzilla.redhat.com/show_bug.cgi?id=502026 . It delays > allocation if possible (as suggested in > http://lists.gnu.org/archive/html/bug-coreutils/2009-05/msg00223.html ), > skipping it altogether if dd is not being used for actual copying.
Hi Ondrej, thanks for working on this. > * src/dd.c (dd_copy): Move and split the code for buffer > allocations to separate functions and call them conditionally. > (alloc_ibuf): Allocate memory for the input buffer. > (alloc_obuf): Allocate memory for the output buffer. Please describe what you did to alloc_ibuf + sibling: you added the function, so please tell so, e.g. something like * src/dd.c (alloc_ibuf, alloc_obuf): Add new function with code factored out from ... (dd_copy): ... here. Call the above new functions to allocate memory for ibuf and obuf at the beginning if needed, or later. And please add also the new definition of ibuf as a global, static variable. > diff --git a/src/dd.c b/src/dd.c > [...] > + /* Delay buffer allocation if possible*/ s/\(possible\)/\1. / > diff --git a/tests/dd/no-allocate.sh b/tests/dd/no-allocate.sh > [...] > +#count and skip is zero, we don't need to allocate memory for input block > +(ulimit -v 10000;dd if=/dev/zero of=x bs=10M count=0) || fail=1 > +#non-skippable input, we need to allocate input block size (and we should > fail) > +(ulimit -v 10000; echo "abcde" | dd of=x bs=10M seek=1 skip=1 count=0) && > fail=1 s/#/# / require_ulimit_ Other than that, the patch looks quite okay - although I needed some time to find out if there could be a situation where ibuf is not yet allocated and alloc_obuf lets obuf point to ibuf, but obuf is yet used before ibuf is allocated later ... and I'm still not 100% convinced. Can someone confirm this? (At least, this corner case is not obvious by reading.) Have a nice day, Berny
