On Monday 12 October 2009 01:36, Gabor Heja wrote:
> 
> Hi all,
> 
> I have attached a patch that adds a third status line with transferred
> bytes, transferred amount of data in human readable format, elapsed seconds
> and speed of transfer for the output of the dd applet, like the one in the
> Debian's coreutils package. Actually its behaviour differs from the
> coreutils' one: it does not change the unit dynamically, instead it always
> uses megabytes for human readable format and speed.
> 
> The patch fixes a possible typo in the dd's part of Config.in: the help
> shows the status lines as one, however the real applet displays the in and
> out blocks in separate lines.
> 
> If anything should be modified in the patch, or I made something wrong,
> please let me know.

+#ifdef ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+       long long unsigned now = monotonic_us();
+#endif

Should be #if ENABLE_xxx, not #ifdef. ENABLE_FEATURE_DD_THIRD_STATUS_LINE
is always defined, to 0 or 1.


        else if (n) /* > 0 */
                G.out_part++;
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+               G.total_bytes += n;
+#endif

wrong indentation.


+                       G.total_bytes, G.total_bytes / 1024 / 1024,

It's a signed division. gcc won't optimize it into shift.


+                       (now - G.begin_time) / 1000000.0,
+                       (G.total_bytes / 1024 / 1024) / ((now - G.begin_time) / 
1000000.0));

(1) are you sure gcc will reuse (now - G.begin_time) / 1000000.0 ?
(2) what if it will be 0?
(3) you lose a lot of precision by dividing mb / sec instead of bytes / 
millisec.


How about attached patch? Tests:


# ./busybox dd </dev/null >/dev/null
0+0 records in
0+0 records out
0 bytes (0B) copied, 0.000006 seconds, 0B/s

# ./busybox dd bs=1M count=2000 </dev/zero >/dev/null
2000+0 records in
2000+0 records out
2097152000 bytes (2.0GB) copied, 1.227030 seconds, 2.0GB/s

# (echo DONE) | ./busybox dd >/dev/null
0+1 records in
0+1 records out
5 bytes (5B) copied, 0.000014 seconds, 365.7KB/s

# (sleep 1; echo DONE) | ./busybox dd >/dev/null
0+1 records in
0+1 records out
5 bytes (5B) copied, 0.999956 seconds, 5B/s

Bloatcheck:

function                                             old     new   delta
dd_output_status                                      68     289    +221
dd_main                                             1463    1482     +19
write_and_stats                                       64      75     +11
packed_usage                                       26529   26526      -3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/1 up/down: 251/-3)            Total: 248 bytes

I committed it to git.

--
vda
diff -d -urpN busybox.7/coreutils/Config.in busybox.8/coreutils/Config.in
--- busybox.7/coreutils/Config.in	2009-10-10 00:46:47.000000000 +0200
+++ busybox.8/coreutils/Config.in	2009-10-14 00:08:42.000000000 +0200
@@ -127,7 +127,16 @@ config FEATURE_DD_SIGNAL_HANDLING
 
 	  $ dd if=/dev/zero of=/dev/null&
 	  $ pid=$! kill -USR1 $pid; sleep 1; kill $pid
-	  10899206+0 records in 10899206+0 records out
+	  10899206+0 records in
+	  10899206+0 records out
+
+config FEATURE_DD_THIRD_STATUS_LINE
+	bool "Enable the third status line upon signal"
+	default n
+	depends on DD && FEATURE_DD_SIGNAL_HANDLING
+	help
+	  Displays a coreutils-like third status line with transferred bytes,
+	  elapsed time and speed.
 
 config FEATURE_DD_IBS_OBS
 	bool "Enable ibs, obs and conv options"
diff -d -urpN busybox.7/coreutils/dd.c busybox.8/coreutils/dd.c
--- busybox.7/coreutils/dd.c	2009-10-10 00:46:47.000000000 +0200
+++ busybox.8/coreutils/dd.c	2009-10-14 00:28:15.000000000 +0200
@@ -34,6 +34,10 @@ static const struct suffix_mult dd_suffi
 
 struct globals {
 	off_t out_full, out_part, in_full, in_part;
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+	unsigned long long total_bytes;
+	unsigned long long begin_time_us;
+#endif
 };
 #define G (*(struct globals*)&bb_common_bufsiz1)
 #define INIT_G() do { \
@@ -44,11 +48,50 @@ struct globals {
 
 static void dd_output_status(int UNUSED_PARAM cur_signal)
 {
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+	unsigned long long total;
+	unsigned long long diff_scaled;
+	unsigned long long diff_us = monotonic_us(); /* before fprintf */
+#endif
+
 	/* Deliberately using %u, not %d */
 	fprintf(stderr, "%"OFF_FMT"u+%"OFF_FMT"u records in\n"
 			"%"OFF_FMT"u+%"OFF_FMT"u records out\n",
 			G.in_full, G.in_part,
 			G.out_full, G.out_part);
+
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+	fprintf(stderr, "%llu bytes (%sB) copied, ",
+			G.total_bytes,
+			/* show fractional digit, use suffixes */
+			make_human_readable_str(G.total_bytes, 1, 0)
+	);
+	/* Corner cases:
+	 * ./busybox dd </dev/null >/dev/null
+	 * ./busybox dd bs=1M count=2000 </dev/zero >/dev/null
+	 * (echo DONE) | ./busybox dd >/dev/null
+	 * (sleep 1; echo DONE) | ./busybox dd >/dev/null
+	 */
+	diff_us -= G.begin_time_us;
+	/* We need to calculate "(total * 1M) / usec" without overflow.
+	 * this would work too, but is bigger than integer code below.
+	 * total = G.total_bytes * (double)(1024 * 1024) / (diff_us ? diff_us : 1);
+	 */
+	diff_scaled = diff_us;
+	total = G.total_bytes;
+	while (total > MAXINT(unsigned long long) / (1024 * 1024)) {
+		total >>= 1;
+		diff_scaled >>= 1;
+	}
+	total *= (1024 * 1024);
+	if (diff_scaled > 1)
+		total /= diff_scaled;
+	fprintf(stderr, "%f seconds, %sB/s\n",
+			diff_us / 1000000.0,
+			/* show fractional digit, use suffixes */
+			make_human_readable_str(total, 1, 0)
+	);
+#endif
 }
 
 static ssize_t full_write_or_warn(const void *buf, size_t len,
@@ -70,13 +113,16 @@ static bool write_and_stats(const void *
 		G.out_full++;
 	else if (n) /* > 0 */
 		G.out_part++;
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+	G.total_bytes += n;
+#endif
 	return 0;
 }
 
 #if ENABLE_LFS
-#define XATOU_SFX xatoull_sfx
+# define XATOU_SFX xatoull_sfx
 #else
-#define XATOU_SFX xatoul_sfx
+# define XATOU_SFX xatoul_sfx
 #endif
 
 int dd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
@@ -157,10 +203,6 @@ int dd_main(int argc UNUSED_PARAM, char 
 	INIT_G();
 	//fflush(NULL); - is this needed because of NOEXEC?
 
-#if ENABLE_FEATURE_DD_SIGNAL_HANDLING
-	signal_SA_RESTART_empty_mask(SIGUSR1, dd_output_status);
-#endif
-
 	for (n = 1; argv[n]; n++) {
 		int what;
 		char *val;
@@ -246,6 +288,14 @@ int dd_main(int argc UNUSED_PARAM, char 
 		flags |= FLAG_TWOBUFS;
 		obuf = xmalloc(obs);
 	}
+
+#if ENABLE_FEATURE_DD_SIGNAL_HANDLING
+	signal_SA_RESTART_empty_mask(SIGUSR1, dd_output_status);
+#endif
+#if ENABLE_FEATURE_DD_THIRD_STATUS_LINE
+	G.begin_time_us = monotonic_us();
+#endif
+
 	if (infile != NULL)
 		xmove_fd(xopen(infile, O_RDONLY), ifd);
 	else {
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to