Olivier Delhomme <[EMAIL PROTECTED]> wrote:
>
[25 lines of unnecessary context removed -- please trim context of
quoted messages]
> Thank you for replying so quickly,
> sorry for that mistake, i'm so confused. Here is the patch
> against today's cvs sources :
Thank you for the patch.
Have you read the GNU Coding Standards document?
Run `info standards' on a Linux system. There are
some minor syntactic problems below. Most would be eliminated
if you used GNU indent output as a model. If you use emacs,
using cc-mode helps for indentation.
This change is large enough that you'll have to send
a copyright assignment to the FSF. Here are the instructions:
It's a two-step process. First, you send some info to the FSF, then
we mail you the actual paperwork to sign. Assignments have to be done
on physical paper to satisfy the requirements of copyright law. When you
fill out the form, the answer to the "employer" question depends on
whether you intend to use any of the old code.
Please email the following information to [EMAIL PROTECTED], and we
will send you the assignment form for your past and future changes.
Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
[What is the name of the program or package you're contributing to?]
[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
[Do you have an employer who might have a basis to claim to own
your changes? Do you attend a school which might make such a claim?]
[For the copyright registration, what country are you a citizen of?]
[What year were you born?]
[Please write your email address here.]
[Please write your postal address here.]
[Which files have you changed so far, and which new files have you written
so far?]
Following are a lot of comments.
I've included most of those suggested changes in an alternative patch, below.
Note that I would like `display=human' to work like --human
on other tools (without requiring that dbs=N be specified),
so I added a little code. This makes it so dbs=N specified
before `display=human' is *ignored*. But that is at least consistent
with du: The -B1 is ignored here:
$ du -B1 -h -s dd.c
40K dd.c
but honored here:
$ du -h -B1 -s dd.c
40960 dd.c
Jim
> --- coreutils/src/dd.c 2003-11-05 23:11:31.000000000 +0100
> +++ coreutils-m/src/dd.c 2003-11-08 21:26:53.000000000 +0100
> @@ -35,6 +35,7 @@
> #include "quote.h"
> #include "safe-read.h"
> #include "xstrtol.h"
Our convention is to alphabetize these #include lines.
...
> -print_stats (void)
> +print_human_stats(void)
There should be a space before the `('.
> {
> - char buf[2][INT_BUFSIZE_BOUND (uintmax_t)];
> + char display_buf[5][MAX (LONGEST_HUMAN_READABLE + 1, INT_BUFSIZE_BOUND
> (uintmax_t))];
No line should be longer than 80 bytes.
> +
> fprintf (stderr, _("%s+%s records in\n"),
Isn't this printing the number of *bytes*, now,
rather than records?
> - umaxtostr (r_full, buf[0]), umaxtostr (r_partial, buf[1]));
> + human_readable (r_full, display_buf[0], human_output_opts, input_blocksize,
> display_block_size),
Likewise.
> + human_readable (r_partial, display_buf[1], human_output_opts,
> input_blocksize, display_block_size));
> +
> fprintf (stderr, _("%s+%s records out\n"),
> - umaxtostr (w_full, buf[0]), umaxtostr (w_partial, buf[1]));
> + human_readable (w_full, display_buf[2], human_output_opts,
> output_blocksize , display_block_size),
> + human_readable (w_partial, display_buf[3], human_output_opts,
> output_blocksize, display_block_size));
> +
> if (r_truncate > 0)
> {
> fprintf (stderr, "%s %s\n",
> - umaxtostr (r_truncate, buf[0]),
> - (r_truncate == 1
> - ? _("truncated record")
> + human_readable (r_truncate, display_buf[5], human_output_opts,
> output_blocksize, display_block_size),
Whoops! The above [5] should be [4].
> + (r_truncate == 1
> ? _("truncated record")
Yow. That's a very long line.
...
> +static void
> +print_stats (void)
> +{
> + char buf[2][INT_BUFSIZE_BOUND (uintmax_t)];
The above declaration can be moved `down' into the scope where it's used.
> +
> + if (STREQ (display,"human"))
always include a space after `,'.
> + { /* human readable format */
> + print_human_stats();
> + }
> + else if (!STREQ (display,"quiet"))
Likewise.
> + { /* in case dd should not be quiet */ <<<
Personally, I require that there be no trailing blanks.
> + fprintf (stderr, _("%s+%s records in\n"),
> + umaxtostr (r_full, buf[0]), umaxtostr (r_partial, buf[1]));
> + fprintf (stderr, _("%s+%s records out\n"),
> + umaxtostr (w_full, buf[0]), umaxtostr (w_partial, buf[1]));
> + if (r_truncate > 0)
> + {
> + fprintf (stderr, "%s %s\n",
> + umaxtostr (r_truncate, buf[0]),
> + (r_truncate == 1
> + ?_("truncated record")
Nearly all operators should be followed by a space,
so there should be a space after `?'.
...
> + else if (STREQ (name, "display")) // choose your display mode (quiet, human,
> normal)
Don't use `//'. That's for C++.
This is C.
> + display = val;
You'll need to add code to diagnose an invalid value,
e.g. `display=wrong'.
...
> + human_output_opts = human_options (getenv ("DD_DISPLAY_BLOCK_SIZE"), false,
> + &display_block_size);
The continued line should be aligned with the opening `('.
> /* Don't close stdout on exit from here on. */
> closeout_func = NULL;
>
And, of course, any time someone adds a new feature,
it makes it easier for me if they also include a patch
for the texinfo documentation.
Here's a patch with most of the above changes:
[I've checked it in on the display_human branch of dd.c,
pending receipt by FSF of the necessary copyright paperwork]
Index: dd.c
===================================================================
RCS file: /fetish/cu/src/dd.c,v
retrieving revision 1.152
diff -u -p -r1.152 dd.c
--- dd.c 5 Nov 2003 03:53:19 -0000 1.152
+++ dd.c 9 Nov 2003 09:17:03 -0000
@@ -30,6 +30,7 @@
#include "error.h"
#include "full-write.h"
#include "getpagesize.h"
+#include "human.h"
#include "inttostr.h"
#include "long-options.h"
#include "quote.h"
@@ -143,6 +144,39 @@ static size_t oc = 0;
/* Index into current line, for `conv=block' and `conv=unblock'. */
static size_t col = 0;
+enum Display_mode
+{
+ /* Generate the default, standards-conforming, style of output:
+ $ dd if=/dev/zero of=/dev/null count=10
+ 10+0 records in
+ 10+0 records out */
+ DISP_DEFAULT,
+
+ /* Generate no such output.
+ $ dd if=/dev/zero of=/dev/null count=10 display=quiet
+ $ */
+ DISP_QUIET,
+
+ /* Generate more readable output:
+ $ dd if=/dev/zero of=/dev/null count=10 display=human
+ 5.0K+0 bytes in
+ 5.0K+0 bytes out
+ You can also specify the display block size:
+ $ dd if=/dev/zero of=/dev/null count=10 display=human dbs=1
+ 5120+0 bytes in
+ 5120+0 bytes out */
+ DISP_HUMAN_READABLE
+};
+
+/* This is aimed to choose diplay type */
+static enum Display_mode display_mode = DISP_DEFAULT;
+
+/* Human-readable options for output. */
+static int human_output_opts;
+
+/* The units to use when printing sizes. */
+static uintmax_t display_block_size;
+
struct conversion
{
char *convname;
@@ -300,11 +334,24 @@ Copy a file, converting and formatting a
of=FILE write to FILE instead of stdout\n\
seek=BLOCKS skip BLOCKS obs-sized blocks at start of output\n\
skip=BLOCKS skip BLOCKS ibs-sized blocks at start of input\n\
+ display=MODE uses display mode according to MODE\n\
+ dbs=SIZE uses SIZE-byte blocks to display statistics\n\
"), stdout);
fputs (HELP_OPTION_DESCRIPTION, stdout);
fputs (VERSION_OPTION_DESCRIPTION, stdout);
fputs (_("\
\n\
+MODE may be:\n\
+ quiet do not display statistics\n\
+ human display statistics in a human readable format\n\
+\n\
+SIZE may be:\n\
+ human prints all sizes in human readable format (e.g. 1K, 234M)\n\
+ si likewise, but uses powers of 1000 instead of 1024\n\
+ BYTES likewise, but use powers of BYTES\n\
+"),stdout);
+ fputs (_("\
+\n\
BLOCKS and BYTES may be followed by the following multiplicative suffixes:\n\
xM M, c 1, w 2, b 512, kB 1000, K 1024, MB 1000*1000, M 1024*1024,\n\
GB 1000*1000*1000, G 1024*1024*1024, and so on for T, P, E, Z, Y.\n\
@@ -366,17 +413,29 @@ bit_count (register int i)
}
static void
-print_stats (void)
+print_human_stats (void)
{
- char buf[2][INT_BUFSIZE_BOUND (uintmax_t)];
- fprintf (stderr, _("%s+%s records in\n"),
- umaxtostr (r_full, buf[0]), umaxtostr (r_partial, buf[1]));
- fprintf (stderr, _("%s+%s records out\n"),
- umaxtostr (w_full, buf[0]), umaxtostr (w_partial, buf[1]));
- if (r_truncate > 0)
+ char display_buf[5][MAX (LONGEST_HUMAN_READABLE + 1,
+ INT_BUFSIZE_BOUND (uintmax_t))];
+
+ fprintf (stderr, _("%s+%s bytes in\n"),
+ human_readable (r_full, display_buf[0], human_output_opts,
+ input_blocksize, display_block_size),
+ human_readable (r_partial, display_buf[1], human_output_opts,
+ input_blocksize, display_block_size));
+
+ fprintf (stderr, _("%s+%s bytes out\n"),
+ human_readable (w_full, display_buf[2], human_output_opts,
+ output_blocksize , display_block_size),
+ human_readable (w_partial, display_buf[3], human_output_opts,
+ output_blocksize, display_block_size));
+
+ if (0 < r_truncate)
{
fprintf (stderr, "%s %s\n",
- umaxtostr (r_truncate, buf[0]),
+ human_readable (r_truncate, display_buf[4],
+ human_output_opts, output_blocksize,
+ display_block_size),
(r_truncate == 1
? _("truncated record")
: _("truncated records")));
@@ -384,6 +443,31 @@ print_stats (void)
}
static void
+print_stats (void)
+{
+ if (display_mode == DISP_HUMAN_READABLE)
+ {
+ print_human_stats ();
+ }
+ else if (display_mode == DISP_DEFAULT)
+ {
+ char buf[2][INT_BUFSIZE_BOUND (uintmax_t)];
+ fprintf (stderr, _("%s+%s records in\n"),
+ umaxtostr (r_full, buf[0]), umaxtostr (r_partial, buf[1]));
+ fprintf (stderr, _("%s+%s records out\n"),
+ umaxtostr (w_full, buf[0]), umaxtostr (w_partial, buf[1]));
+ if (r_truncate > 0)
+ {
+ fprintf (stderr, "%s %s\n",
+ umaxtostr (r_truncate, buf[0]),
+ (r_truncate == 1
+ ? _("truncated record")
+ : _("truncated records")));
+ }
+ }
+}
+
+static void
cleanup (void)
{
print_stats ();
@@ -575,6 +659,26 @@ scanargs (int argc, char **argv)
output_file = val;
else if (STREQ (name, "conv"))
parse_conversion (val);
+ else if (STREQ (name, "display")) /* select display mode */
+ {
+ if (STREQ (val, "human"))
+ {
+ display_mode = DISP_HUMAN_READABLE;
+ human_output_opts = human_autoscale | human_SI | human_base_1024;
+ display_block_size = 1;
+ }
+ else if (STREQ (val, "quiet"))
+ {
+ display_mode = DISP_QUIET;
+ }
+ else
+ {
+ error (0, 0, _("unrecognized display= argument %s"), quote (val));
+ usage (EXIT_FAILURE);
+ }
+ }
+ else if (STREQ (name, "dbs")) /* select display block size */
+ human_output_opts = human_options (val, true, &display_block_size);
else
{
int invalid = 0;
@@ -1163,6 +1267,9 @@ main (int argc, char **argv)
parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE, VERSION,
usage, AUTHORS, (char const *) NULL);
+ human_output_opts = human_options (getenv ("DD_DISPLAY_BLOCK_SIZE"),
+ false, &display_block_size);
+
/* Don't close stdout on exit from here on. */
closeout_func = NULL;
_______________________________________________
Bug-coreutils mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-coreutils
