On Wed, 2 Apr 2014, Xin Li wrote:

On 4/2/14, 9:45 PM, Bruce Evans wrote:
On Thu, 3 Apr 2014, Xin LI wrote:
0x00020 -#define    C_FILES        0x00040 -#define    C_IBS
0x00080 -#define    C_IF        0x00100 -#define    C_LCASE
0x00200 -#define    C_NOERROR    0x00400 -#define    C_NOTRUNC
0x00800 -#define    C_OBS        0x01000 -#define    C_OF
0x02000 -#define    C_OSYNC        0x04000 -#define    C_PAREVEN
0x08000 -#define    C_PARNONE    0x100000

The previous formatting was not too good, and obfuscated a bug
here. 4 bits are unused.  Someone added this bug together with many
others in 2004, and I missed it then when I committed a fix for the
others. Most of the bugs that I fixed were unsorting of the table
by adding to its end.  Insertion sort tends to give large churn of
the table by changing all the numbers, but for the 2004 fix there
wasn't much churn except relative to the unsorted version because
all the changes were near the end.

Actually it was me who added the unused bits.  *Blush*.

I see you committed a fix.

-#define    C_PARODD    0x200000 -#define    C_PARSET
0x400000 -#define    C_SEEK        0x800000 -#define    C_SKIP
0x1000000 -#define    C_SPARSE    0x2000000 -#define    C_SWAB
0x4000000 -#define    C_SYNC        0x8000000 -#define    C_UCASE

Mail programs keep getting worse.  Here they destroyed the line wrapping
together with rearranging the quotes.

Will it be an improvement if we have the attached change committed?
Basically it redo the sort and use 1 <<'s instead of hardcoded heximal
numbers.

Too much style changing.  dd still uses hex constants for smaller flag
values elsewhere, even in the same file.

The 2 values above UINT_MAX also won't compile with compilers that
warn that 'integer constant is too large for "long" type'.  Mainly
gcc
without
-std=c99.  clang is too incompatible to warn about this even with
-std=c89 -Wall.  The fix is not to further churn the table by
adding a ULL suffix to all entries.

Using shifts gives a similar type problem for 32-bit types.  From the
patch:

% -#define      C_NOXFER        0x10000000
% -#define      C_NOINFO        0x20000000
% ...
% +#define      C_UCASE         (1 << 28)
% +#define      C_UNBLOCK       (1 << 29)

This works up to (1 << 30), assuming 32-bit ints, but (1 << 31) would
overflow.  It would have to be written as (1U << 31) (keep assuming
32-bit ints), but this would mess up the fancy formatting.  Elswhere
in the patch:

% -#define      C_ASCII         0x00000001
% -#define      C_BLOCK         0x00000002
% ...
% +#define      C_ASCII         (1 <<  0)
% +#define      C_BLOCK         (1 <<  1)

Fancy formatting like this (the extra space before the shift count to line
things up) is hard to maintain since it is too fancy for indent(1) to produce
or preserve.

Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to