[sane-devel] [PATCH 1/2] use getopt()
Matteo Croce
matteo at openwrt.org
Sun Sep 13 16:11:05 UTC 2015
2015-09-13 7:18 GMT+02:00 Olaf Meeuwissen <paddy-hack at member.fsf.org>:
> Matteo Croce writes:
>
>> Sorry about that, I'll fix in a new version, also what do you think
>> about a nicer help string like:
>> "Usage: %s [-a username] [-d n] [-s n] [-h]\n"
>
> Your suggestion made me take a second look and I noticed that I totally
> missed the fact that the usage message treats the options as mutually
> exclusive with optional arguments. On top of that, the manual page has
> a slightly different idea of the usage yet. Your code changes would
> accept any number of the options and require arguments for three of
> them.
>
> Code's usage: [ -a [ username ] | -d [ n ] | -s [ n ] ] | -h
> Manual page : [ -a [ username ] | -d [ n ] | -s [ n ] | -h ]
> getopt : [-a username] [-d n] [-s n] [-h]
>
> An incompatible change like that of course cannot go in. I liked the
> explicit passing of username to run_standalone() and an optional socket
> to run_inetd() though. It better to have the argv handling all in one
> function. If you could split that from the getopt() part, that'd be
> nice. AFAIK, getopt() does not support mutually exclusive options so
> you'd have to add checking for that after the command-line parsing.
>
> Let's have a look at what the code on master accepts
>
> - a plain `saned`
> - `saned -a [username]`. Any trailing arguments are silently ignored.
> The optional username is used by run_standalone or run_inetd. For an
> invocation like `saned -a -d`, the username would be `-d`. Oops!
> - `sane -d[n]`. Again any trailing arguments are silently ignored. An
> unquoted space between the -d and the optional n will cause n to be
> ignored.
> - for `saned -s[n]` the situation is the same as `saned -d[n]`
> - `saned -h` and `saned --help`
>
> and on OS/2 it also accepts
>
> - `saned socketnumber`
>
> but only if there are no other command-line arguments.
>
> So that's different again from what the usage message and manual page
> claim. Yuck! A usage message (and manual page synopsis) like
>
> saned [-h | --help]
> saned -a [username]
> saned -d[n]
> saned -s[n]
> saned socketnumber # On OS/2 only
>
> may be better.
It's not an incompatible change, actual use cases will still work the same:
- plain `saned` will work as inetd
- `saned -a [username]` will work as standalone as user username, with
the only difference that now `saned -a -d` works as expected
- `sane -d[n]` still works, but an extra space is permitted
- same for `sane -s[n]`
There is no point in having -a and -s mutually exclusive, syslog
debugging level should be configurable even in standalone mode.
The same applies to -d and -s, if you specify "-d -s" (but why someone
should) with current master it will output to stderr,
and the same applies with my patch as 's' is the same case without break.
>> The main issue is that it seems that under OS/2 the socket handle is
>> passed as an extra argument, what to do?
>> Handle the extra argument only if HAVE_OS2_H is defined?
>
> I'd a run_inetd(char *sock) everywhere and pass NULL if not on OS/2.
> That would get rid of the #ifdef'd function signature. In main() you
> can then use something like
nice, I will change it
> /*...*/
> else {
> char *sock = NULL;
>
> #ifdef HAVE_OS2_H
> if (argc == 2) sock = argv[1];
> #endif
> run_inetd(sock);
> }
>
>> BTW is OS/2 still used? The Sane OS/2 mailing list is gone, and the
>> latest binary was built on 2012.
>
> I have no idea but as long as supporting it is as simple as above there
> no reason to drop it. Is there?
>
> Hope this helps,
> --
> Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27
> Support Free Software Support the Free Software Foundation
> https://my.fsf.org/donate https://my.fsf.org/join
--
Matteo Croce
OpenWrt Developer
_______ ________ __
| |.-----.-----.-----.| | | |.----.| |_
| - || _ | -__| || | | || _|| _|
|_______|| __|_____|__|__||________||__| |____|
|__| W I R E L E S S F R E E D O M
-----------------------------------------------------
CHAOS CALMER
-----------------------------------------------------
* 1 1/2 oz Gin Shake with a glassful
* 1/4 oz Triple Sec of broken ice and pour
* 3/4 oz Lime Juice unstrained into a goblet.
* 1 1/2 oz Orange Juice
* 1 tsp. Grenadine Syrup
-----------------------------------------------------
2015-09-13 7:18 GMT+02:00 Olaf Meeuwissen <paddy-hack at member.fsf.org>:
> Matteo Croce writes:
>
>> Sorry about that, I'll fix in a new version, also what do you think
>> about a nicer help string like:
>> "Usage: %s [-a username] [-d n] [-s n] [-h]\n"
>
> Your suggestion made me take a second look and I noticed that I totally
> missed the fact that the usage message treats the options as mutually
> exclusive with optional arguments. On top of that, the manual page has
> a slightly different idea of the usage yet. Your code changes would
> accept any number of the options and require arguments for three of
> them.
>
> Code's usage: [ -a [ username ] | -d [ n ] | -s [ n ] ] | -h
> Manual page : [ -a [ username ] | -d [ n ] | -s [ n ] | -h ]
> getopt : [-a username] [-d n] [-s n] [-h]
>
> An incompatible change like that of course cannot go in. I liked the
> explicit passing of username to run_standalone() and an optional socket
> to run_inetd() though. It better to have the argv handling all in one
> function. If you could split that from the getopt() part, that'd be
> nice. AFAIK, getopt() does not support mutually exclusive options so
> you'd have to add checking for that after the command-line parsing.
>
> Let's have a look at what the code on master accepts
>
> - a plain `saned`
> - `saned -a [username]`. Any trailing arguments are silently ignored.
> The optional username is used by run_standalone or run_inetd. For an
> invocation like `saned -a -d`, the username would be `-d`. Oops!
> - `sane -d[n]`. Again any trailing arguments are silently ignored. An
> unquoted space between the -d and the optional n will cause n to be
> ignored.
> - for `saned -s[n]` the situation is the same as `saned -d[n]`
> - `saned -h` and `saned --help`
>
> and on OS/2 it also accepts
>
> - `saned socketnumber`
>
> but only if there are no other command-line arguments.
>
> So that's different again from what the usage message and manual page
> claim. Yuck! A usage message (and manual page synopsis) like
>
> saned [-h | --help]
> saned -a [username]
> saned -d[n]
> saned -s[n]
> saned socketnumber # On OS/2 only
>
> may be better.
>
>> The main issue is that it seems that under OS/2 the socket handle is
>> passed as an extra argument, what to do?
>> Handle the extra argument only if HAVE_OS2_H is defined?
>
> I'd a run_inetd(char *sock) everywhere and pass NULL if not on OS/2.
> That would get rid of the #ifdef'd function signature. In main() you
> can then use something like
>
> /*...*/
> else {
> char *sock = NULL;
>
> #ifdef HAVE_OS2_H
> if (argc == 2) sock = argv[1];
> #endif
> run_inetd(sock);
> }
>
>> BTW is OS/2 still used? The Sane OS/2 mailing list is gone, and the
>> latest binary was built on 2012.
>
> I have no idea but as long as supporting it is as simple as above there
> no reason to drop it. Is there?
>
> Hope this helps,
> --
> Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27
> Support Free Software Support the Free Software Foundation
> https://my.fsf.org/donate https://my.fsf.org/join
--
Matteo Croce
OpenWrt Developer
_______ ________ __
| |.-----.-----.-----.| | | |.----.| |_
| - || _ | -__| || | | || _|| _|
|_______|| __|_____|__|__||________||__| |____|
|__| W I R E L E S S F R E E D O M
-----------------------------------------------------
CHAOS CALMER
-----------------------------------------------------
* 1 1/2 oz Gin Shake with a glassful
* 1/4 oz Triple Sec of broken ice and pour
* 3/4 oz Lime Juice unstrained into a goblet.
* 1 1/2 oz Orange Juice
* 1 tsp. Grenadine Syrup
-----------------------------------------------------
More information about the sane-devel
mailing list