Matteo Croce writes: > 2015-09-13 7:18 GMT+02:00 Olaf Meeuwissen <[email protected]>: >> 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 -- sane-devel mailing list: [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel Unsubscribe: Send mail with subject "unsubscribe your_password" to [email protected]
