svn commit: r230037 - head/lib/libutil
Bruce Evans
brde at optusnet.com.au
Sat Jan 14 21:02:35 UTC 2012
On Sat, 14 Jan 2012, Pawel Jakub Dawidek wrote:
> On Sat, Jan 14, 2012 at 09:59:27PM +1100, Bruce Evans wrote:
>> ...
>> It's good to declare mode_t, since pidfile_open() uses it and we want
>> to remove the dependency on <sys/param.h>. However, this definition
>> doesn't follow KNF or the style of all the other typedef declarations
>> in the file, since all the others follow KNF and thus have a space
>> instead of a tab after #define and also after typedef.
>
> I think you mixed space with tab. All the others have a tab after
> #define and typedef. I fully agree this should be consistent.
Oops.
>>> -#ifdef _SYS_PARAM_H_
>>> int pidfile_close(struct pidfh *_pfh);
>>> int pidfile_fileno(const struct pidfh *_pfh);
>>> struct pidfh *
>>> pidfile_open(const char *_path, mode_t _mode, pid_t *_pidptr);
>>> int pidfile_remove(struct pidfh *_pfh);
>>> int pidfile_write(struct pidfh *_pfh);
>>> -#endif
>>
>> Now these are unsorted, since a separate section to hold them is not
>> needed. It was used just to make the ifdef easier to read (we don't
>> want to split up the main list with blank lines around each ifdef, and
>> without such blank lines the ifdefs are harder to read).
>
> I'd prefer not to change that. All those functions are part of pidfile(3)
> API and it would be better, IMHO, to keep them together here too.
The functions have a unique prefix, so they are grouped nicely when sorted
into a long list.
While I'm here, I'll complain about the verboseness of that prefix :-).
Other APIs in the file mostly use short prefixes:
- kinfo_. Should have been ki_ like its struct member names. pidfile uses
a good prefix for its struct member names too.
- properties_/property_. Bad, like the rest of the API.
- uu_. A weird nondescriptive name for serial device locking protocol.
Is it from uucp? But its weirdness makes it memorable, unlike a
generic English word like `property'. Better yet, I don't have to
quote it here.
- f. Stdio's prefix meaning `file'. To fit indentifiers in 8 characters,
it can't even have an underscore.
- pw_. Old prefix/abbrieviation for `password'. It's more readable than
`password' once you are used to it.
- gr_. Newer prefix for `group'. More verbose than the g in gid.
- quota_. At least the English word is short.
Just noticed some more disorder: the groups of the defines at the end
are in random (mostly historical) order (U*, HO*, F*, PW*, HN* (for
the last parameter of humanize_number()), HN* (for the second last
parameter...), HD*.
If the pidfile API had defines and if the API is kept in its own
section, its defines should be in that section. Most of the other APIs
that have a man page are large enough to deserve the same treatment
if it is done for pidfile. Some like dehumanize^Wscientificize^W
humanize_number() are larger although they have fewer functions, since
they have lots of defines.
Bruce
More information about the svn-src-all
mailing list