Re: [PATCH v2 05/10] archive-tar: allow to accumulate writes before writing 512-byte blocks

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: May 2, 2012, 14:28 UTC
Message-ID: <4FA14497.5090203@lsrfire.ath.cx>
In-Reply-To: <1335965122-17458-6-git-send-email-pclouds@gmail.com>

Am 02.05.2012 15:25, schrieb Nguyễn Thái Ngọc Duy:

Show 13 quoted lines

> 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.

 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).