On Tue, Sep 13, 2011 at 11:13:57PM +0200, Sami Kerola wrote:
> I started to look sys-utils directory, and got obsessed by getting ipc
> stuff to behave like a tool from this millennium. The whole directory
> is not done, but I feel this is already big enough patch set.
Some notes about USAGE_*:
* USAGE_BEGIN_TAIL is unnecessary if we know that USAGE_MAN_TAIL is
always on new line, we can use \n at the begin of the
USAGE_MAN_TAIL:
"\nFor more details see %s.\n"
* I don't like the current
#define USAGE_MAN_TAIL _("For more details see %s.\n")
macro. It's bad manner to use hidden argument(s) for macros, it would
be better to have:
#define USAGE_MAN_TAIL(_man) _("\nFor more details see %s.\n"), _man
* how we want to use USAGE_HELP and USAGE_VERSION macros? The strings
in the macros are the same for all programs, it means that there is
the same space between "-h, --help" and "display this help and exit"
in all programs. For example:
printf(_("-x, --extra-mega-cool <file> enable the best feature for <file>\n");
printf(USAGE_HELP);
printf(USAGE_USAGE);
-x, --extra-mega-cool <file> enable the best feature for <file>
-h, --help display this help and exit
-V, --version output version information and exit
mess, maybe we need an extra line between program specific options
and the common options:
printf(_("-x, --extra-mega-cool <file> enable the best feature for <file>\n");
printf(USAGE_SEPARATOR);
printf(USAGE_HELP);
printf(USAGE_USAGE);
printf(USAGE_MAN_TAIL("foo(1)");
output:
-x, --extra-mega-cool <file> enable the best feature for <file>
-h, --help display this help and exit
-V, --version output version information and exit
For more details see foo(1).
it also means that --help and --version should be at the end of the
options list.
> setarch: move options struct to function scope
> setarch: use program_invocation_short_name
> setarch: add version printing
> ipcmk: add long options & fix usage()
> ipcmk: remove useless code
> ipcmk: validate numeric option arguments
> ipcmk: remove camel casing
> ipcmk: include-what-you-use header check
Note that in the 2.20 release we have introduced a new bug by
include-what-you-use patch. It's necessary to test it with
(at least) --disable-nls.
The ideal solution would be to add something like tools/checkbuilds
to test different ./configure scenarios (enable/disable NLS,
slang/ncurses/ncursesw, ...). Now it's possible that not all
scenarios are tested now. We have to test it, because people are too
lazy to test -rc releases...
> ipcrm: add long options
> ipcrm: exit if unknown error occurs
Please, don't use "else" after non-return functions. For example:
if (x)
non_return_func()
else
...
Heee.. smatch should be improved to detect this code :-)
> ipcrm: refactor new and old main to share code
Please:
- check strtoul() result. See strutils.c
- if possible, then use only one line for var = E ? x : y; it's
unnecessary to move ": y" to another line.
- don't use "error" as variable name, glibc provides such function.
- btw, do we have regression test for ipcrm? :-)
> ipcrm: include-what-you-use header check
> ipcs: add long options
> ipcs: include-what-you-use header check
> ipcs: comment & white space clean up
> pivot_root: add version & help option
> docs: mention long options in pivot_root.8
Hmm... there should be also switch_root(8) in SEE ALSO section.
> ctrlaltdel: add version & help options
> docs: mention long options in ctrlaltdel.8
> docs: add --version to setarch.8
> docs: add long options to ipcmk.1 man page
> docs: add long options to ipcrm.1 man page
> docs: add long options to ipcs.1 man page
> ipcrm: add --all option
> ipcmk: allow high speed ipc creation
> ipcrm: add --verbose option
...
> 13 files changed, 788 insertions(+), 564 deletions(-)
Thanks!
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
