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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
