Re: [PATCH] delaying dd allocation

[Bernhard Voelker]

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