From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Wed, 02 May 2012 14:43:08 GMT
Subject: Re: [PATCH v2 05/10] archive-tar: allow to accumulate writes before writing 512-byte blocks
Message-ID: <CACsJy8D5P9EESqwJLqa3Ragixr-TFJby4h4FgqOxm1jC8L9aHg@mail.gmail.com>
In-Reply-To: <4FA14497.5090203@lsrfire.ath.cx>

On Wed, May 2, 2012 at 9:28 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 02.05.2012 15:25, schrieb Nguyễn Thái Ngọc Duy:
>> This allows to split single write_blocked(buf, 123) call into multiple
>> calls
>>
>> write_blocked(buf, 100, 1);
>> write_blocked(buf,  23, 1);
>> write_blocked(buf,   0, 0);
>>
>> No call sites do this yet though.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy<pclouds@gmail.com>
>> ---
>>   archive-tar.c |   16 +++++++++-------
>>   1 files changed, 9 insertions(+), 7 deletions(-)
>
> Hmm, I'm not a fan of adding binary parameters to distinguish between two
> modes of a function.  It's usually better to split it up at that point.
> E.g. the patch below does that and still provides the old interface, i.e.
> the existing callers don't need to be changed.

Good point. Then this can be merged to the next patch too.

>  archive-tar.c |   15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index 20af005..a2babe1 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -30,10 +30,9 @@ static void write_if_needed(void)
>  * queues up writes, so that all our write(2) calls write exactly one
>  * full block; pads writes to RECORDSIZE
>  */
> -static void write_blocked(const void *data, unsigned long size)
> +static void do_write_blocked(const void *data, unsigned long size)
>  {
>        const char *buf = data;
> -       unsigned long tail;
>
>        if (offset) {
>                unsigned long chunk = BLOCKSIZE - offset;
> @@ -54,6 +53,12 @@ static void write_blocked(const void *data, unsigned long size)
>                memcpy(block + offset, buf, size);
>                offset += size;
>        }
> +}
> +
> +static void finish_record(void)
> +{
> +       unsigned long tail;
> +
>        tail = offset % RECORDSIZE;
>        if (tail)  {
>                memset(block + offset, 0, RECORDSIZE - tail);
> @@ -62,6 +67,12 @@ static void write_blocked(const void *data, unsigned long size)
>        write_if_needed();
>  }
>
> +static void write_blocked(const void *data, unsigned long size)
> +{
> +       do_write_blocked(data, size);
> +       finish_record();
> +}
> +
>  /*
>  * The end of tar archives is marked by 2*512 nul bytes and after that
>  * follows the rest of the block (if any).



-- 
Duy