Re: [OT] Is od broken?

[Jim Meyering]

Eric Blake <address@hidden> wrote:
...
>>        /* Don't use %#e; not all systems support it.  */
>> -      pre_fmt_string = " %%%d.%de";
>> +      pre_fmt_string = "";
>
> Unrelated to this patch: Should we use %g instead of %e for floating point?
> Seeing 1.000000000e+00 is somewhat distracting in isolation; on the other 
> hand,
> the variable-width nature of %g might not look as nice as the fixed-width
> precision of %e.

I have a slight preference for the status quo.  And not having
looked at how other vendor od programs work, I'm hesitant to change this.

>> @@ -870,6 +880,7 @@ this system doesn't provide a %lu-byte floating point
> type"),
>>    tspec->print_function = print_function;
>>
>>    tspec->field_width = field_width;
>> +  tspec->pad_width = 0;
>
> I've added more comments on what pad_width represents (particularly since I 
> now
> use it as the total padding to be distributed across the line, such that
> pad_width >= number_of_fields; and not as a pad-per-field).
>
> Here's my latest patch on top of my v2 series, using Paul's ideas.  I miss
> having the ChangeLog in the repository; how do you attribute a patch to
> multiple authors using just the git log entry, short of having two S-O-B 
> lines,
> even though I'm not really supposed to forge a S-O-B for Paul?  The intent is
> to generate a ChangeLog entry that looks like:

This is indeed something that needs to be addressed.

How about adding a specially formatted line to the commit log
that the build-aux/gitlog-to-changelog will then recognize and use
to emit the desired attribution.

> 2008-06-12  Eric Blake  <address@hidden>
>       and Paul Eggert  <address@hidden>
>
>       improve handling of padding
>       * src/od.c (decode_one_format): Alter the format, again.
>       (FMT_BYTES_ALLOCATED): Reduce size by adjusting to new format.
>       (MAX_INTEGRAL_TYPE_SIZE): Move earlier in the file.
>       (charname): Turn it into a 2D array, since there's no need for
>       pointers now.
>       (PRINT_TYPE, print_named_ascii, print_ascii): Add a width
>       parameter.
>       (write_block): Account for width parameter.
>
> Should we squash this on top of the previous patch, or keep it as a separate
> commit?

I think it's fine (and probably better, but haven't reviewed either
carefully yet) to keep them separate.

>>From 5dc083545a668d04ca2c73e34698761af913d2d4 Mon Sep 17 00:00:00 2001
> From: Eric Blake <address@hidden>
> Date: Thu, 12 Jun 2008 11:05:09 -0600
> Subject: [PATCH] improve handling of padding
>
> * src/od.c (decode_one_format): Alter the format, again.
> (FMT_BYTES_ALLOCATED): Reduce size by adjusting to new format.
> (MAX_INTEGRAL_TYPE_SIZE): Move earlier in the file.
> (charname): Turn it into a 2D array, since there's no need for
> pointers now.
> (PRINT_TYPE, print_named_ascii, print_ascii): Add a width
> parameter.
> (write_block): Account for width parameter.
> Using ideas from Paul Eggert.