[sane-devel] [PATCH 1/2] use getopt()

[Olaf Meeuwissen]

Matteo Croce writes:

> 2015-09-13 7:18 GMT+02:00 Olaf Meeuwissen <paddy-hack at member.fsf.org>:
>> Matteo Croce writes:
>>
>> [snip]
>>   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.
>> [snip]
>
> 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

My main concern here is that the code has not had any testing that the
SANE project is aware of for these cases.  The original code was written
against the later options being ignored.  Now all of a sudden that is no
longer the case.

> - `sane -d[n]` still works, but an extra space is permitted

This is not problematic of course.

> - 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.

Logically, combining command-line options may be fine but will the code
still work as one would expect?

Forgetting about the help option and option arguments for a minute, the
original code only had four cases to consider.  Your code has eight.
Have the extra four been validated?  You only mention three in the
above.  What about -a -d -s?

If they have, then that's fine and I have no objections in this going
in.  We just have to remember to mention that the saned command-line has
slightly changed in the NEWS file then.

Sorry to be such a nitpick but saned is a server and that makes me extra
nervous.

>>> 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

Looking forward to updated patches.

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