On Thu, Feb 24, 2011 at 10:39:11PM +0100, Sami Kerola wrote:
> This was TODO item from commit 947a7c9c. The patch also
> introduces version and help switches.
>
> Signed-off-by: Sami Kerola <kerolasa@xxxxxx>
> ---
> text-utils/Makefile.am | 2 +
> text-utils/tailf.c | 86 +++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 69 insertions(+), 19 deletions(-)
Applied with a small change, thanks.
> +long old_style_option(int argc, char **argv)
> +{
> + int i, j;
> + long lines = -1;
> + for (i = j = 0; i < argc; i++) {
> + if (argv[i][0] == '-' && isdigit(argv[i][1])) {
> + lines = strtol_or_err(++argv[i], _("invalid number of lines"));
> + j++;
> + }
> + if (i + j < argc)
> + argv[i] = argv[i+j];
> + }
> + return lines;
> +}
IMHO this function is implemented incorrectly. The idea is to parse "-N"
and remove the option from argv[], right?
- you have to update 'argc' if you removed any option from argv[]
- after argv[i] = argv[i+j], the 'i' is incremented and argv[i] is
not parsed, it's possible that there is again '-N' which will be
visible for getopt()
I think that's better to use memmove() than argv[i] = argv[i+j].
> + while ((ch = getopt_long(argc, argv, "n:N:Vh0123456789", longopts, NULL)) != -1)
> + switch((char)ch) {
> + case 'n':
> + case 'N':
> + lines = strtol_or_err(optarg, _("invalid number of lines"));
> + break;
> + case 'V':
> + printf(_("%s from %s\n"), program_invocation_short_name,
> + PACKAGE_STRING);
> + exit(EXIT_SUCCESS);
> + case 'h':
> + usage(stdout);
> + case '0': case '1': case '2': case '3': case '4':
> + case '5': case '6': case '7': case '8': case '9':
> + errx(EXIT_FAILURE, _("option used in invalid context -- %c"), ch);
This is unnecessary if you have correctly implemented old_style_option().
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
