svn commit: r230037 - head/lib/libutil
brde at optusnet.com.au
Tue Jan 17 08:44:48 UTC 2012
On Mon, 16 Jan 2012, Guy Helmer wrote:
> On Jan 14, 2012, at 3:02 PM, Bruce Evans wrote:
> I've pasted the diff below that I think captures the majority of the issues you have brought up. I have not attempted to tackle the property.3/properties.3 issues, nor the objections to the prefixes that I think would take considerably more effort to resolve -- I wanted to concentrate on the issues that can be isolated to libutil. I hope the diff was pasted OK, especially WRT <tab> characters.
I see you committed it. It is mostly OK, but as usual a pre-commit review
would have been useful.
> Index: lib/libutil/libutil.h
> --- lib/libutil/libutil.h (revision 230221)
> +++ lib/libutil/libutil.h (working copy)
> +/* Flags for hexdump(3) */
> #define HD_COLUMN_MASK 0xff
> #define HD_DELIM_MASK 0xff00
> #define HD_OMIT_COUNT (1 << 16)
> #define HD_OMIT_HEX (1 << 17)
> #define HD_OMIT_CHARS (1 << 18)
> +/* Flags for humanize_number(3) flags */
The double "flags" is meaningful but hard to parse.
> +#define HN_DECIMAL 0x01
> +#define HN_NOSPACE 0x02
> +#define HN_B 0x04
> +#define HN_DIVISOR_1000 0x08
> +#define HN_IEC_PREFIXES 0x10
> +/* Flags for humanize_number(3) scale */
It is only after reading this that the double "flags" starts making
sense. Add " parameter" to the end of each to make more sense. Then
omit the first "Flags " to make even more sense. Then consider adding
some punctuation. Then add "Values" where "Flags" was. The first
"Flags" was only a paraphrase of "`flags' parameter". This is most
useful when the parameter's name is not `flags', and only works then
there is only 1 parameter that takes flags. This results in:
/* Values for humanize_number(3)'s `flags' parameter */
/* Values for humanize_number(3)'s `scale' parameter */
Perhaps you can abbreviate this a bit. "Values " is mostly tautologous
but seems to improve readability.
> +/* return values from realhostname() */
This one is not capitalized.
> +/* Return values from uu_lock() */
> +#define UU_LOCK_INUSE 1
> +#define UU_LOCK_OK 0
> +#define UU_LOCK_OPEN_ERR -1
> +#define UU_LOCK_READ_ERR -2
> +#define UU_LOCK_CREAT_ERR -3
> +#define UU_LOCK_WRITE_ERR -4
> +#define UU_LOCK_LINK_ERR -5
> +#define UU_LOCK_TRY_ERR -6
> +#define UU_LOCK_OWNER_ERR -7
Er, the parentheses were not redundant for -N, since -N consists of 2
tokens. I just tried to find an example of why they are needed in
<float.h>, but could only find historical and contrived examples:
- before C90, they were necessary to prevent - -1 becoming --1. Code
like -UU_LOCK_OPEN_ERR isn't necessarily an error. For a non-flag
macro FOO, -FOO just isn't an error, so FOO needed to be parenthesized
it it was a negative integer.
- now, 1(-1) is a syntax error, but 1-1 is not. Code like
1 UU_LOCK_OPEN_ERR should be a syntax error, but isn't if the macro
expands to -1. This is unimportant compared with -FOO working, since
the parentheses only give detection of an error.
Parentheses are necessary for casts in macros even for C90 and later,
since if FOO is defined as (long)1 then the useful expression
`sizeof FOO' is the syntax error `sizeof(long) 1'.
To avoid avoid worrying about this, expression-like macros should
always be parenthesized iff they have more than 1 token.
When worrying about this, you look at the C operator precedence table
and notice that parentheses don't have the highest precedence, since they
share precedence with some other operators. So there must be some magic
for parenthesizing macros to be sufficient.
More information about the svn-src-all