[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-inetutils] [PATCH 1/7] argp'fied argument parsing & tiny fixes
From: |
Arash Yadegarnia |
Subject: |
Re: [bug-inetutils] [PATCH 1/7] argp'fied argument parsing & tiny fixes |
Date: |
Sat, 30 Aug 2008 00:06:22 +0430 |
On Sat, 2008-08-23 at 05:01 -0400, Alfred M. Szmidt wrote:
> Tool name: ftpd
>
> ftpd/ftpd.c:
>
> * Removed getopt () interface
> * Added argp_* interface
> * Removed usage ()
>
>
> Note that we use GNU ChangLog entries, please check the GNU Coding
> Standards, and the ChangeLog file in inetutils.
* ftpd/ftpd.c: Include <argp.h>. Remove include for <getopt.h>.
(ARGP_PROGRAM_DATA): Call macro.
(argp, args_doc, doc, argp_options): New variables.
(parse_opt): New function.
(usage): Function removed.
(short_options, long_options): Variables removed.
(main): Added new variable INDEX. Use argp to parse program
options.
>
> +const char AUTH_ARG_DESC[] = "Use AUTH for authentication, it can be:\n"
> +"\tdefault (passwd authentication)\n"
> +#ifdef WITH_PAM
> +"\tpam (using pam 'ftp' module)\n"
> +#endif
> +#ifdef WITH_KERBEROS
> +"\tkerberos\n"
> +#endif
> +#ifdef WITH_KERBEROS5
> +"\tkderberos5\n"
> +#endif
> +#ifdef WITH_OPIE
> +"\topie"
> +#endif
> +;
>
> I find this immensly ugly. For one, AUTH_ARG_DESC is not a macro, so
> it should not be upper-case. Secondly, I am guessing that --help will
> look completely messed up. Something simpler would be more useful,
> just a comma seperated list of the options available. Or simply not
> listing them at all, they should be documented in the manual.
Agreed. this is an ugly hack. But since I wasn't sure enough to move
them to the manual, I put them that way. BTW, --help looks OK, even
with those '\t'. Anyways, As you mentioned, I think the best place for them, is
the manual. Strangely, the current man page tells an awkward story for
'-a' which doesn't make any sense, at least for what it is
supposed to do.
I've added some more description about it ('-a') in the man page
which describes the argument itself and all the values it can possess.
However, 3 of 5 options are not implemented in the current code.
'Kerberos', 'Kerberos5' and 'Opie' do nothing but falling back to
'passwd' authentication. I also included them in the manual.
Attached:
* Updated ftpd patch
* man page patch
PS, Is there any plan too add kerberos support to ftpd ? I mean do you
think it will worth the effort ?
Regards,
ftpd.patch
Description: Text Data
man.patch
Description: Text Data