[sane-devel] [PATCH 3/3] saned: reorganize flags, remove run_mode SANED_RUN_DEBUG

[Olaf Meeuwissen]

Hi Luiz,

Thanks for your patches.  I really appreciate that you also keep the
documentation in sync!

Luiz Angelo Daros de Luca writes:

> The two first patches are trivial bugfixes.

Indeed they are and I'll push them shortly.

> However, this one proposes a new organization on saned options, as
> shown at:
>
> https://alioth.debian.org/tracker/index.php?func=detail&aid=315747&group_id=30186&atid=410366

Doing the saned options the Unix way?  Great, "Do just one thing" but
let's also have a look at the "and do it well" part.  This is not to
imply that your patch is not doing things well, btw, just a little
reminder that that is also part of the Unix way.

> All flags now do one thing and normally have an opposite flag that can
> deactivate it.

I am not sure what you are trying to achieve with the opposite flags.
What use case do you have in mind for them?

I can think of:
 - negating an option set in a configuration file
 - negating an earlier option on the command-line
 - signalling a running saned to toggle its behaviour
but none look overly compelling to me.  I even think that the first and
last ones are not supported by saned anyway.

If there is no clear use case, perhaps saned should not provide them.
In that case, I'd remove:
 - -D and make running in the background the default because saned is
   normally meant to run as a daemon
 - -s and make logging to syslog the default because daemon processes
   shouldn't scribble on anyone's terminal unless asked to
 - -l and make running in stand-alone mode the default because that's
   what daemons ought to do anyway

# Hmm, looks like the manual page needs more changes to update the
# configuration sections for (x)inetd and systemd then.

> The flag -a was kept compatible but, flags -d and -s now have a
> different behavior.  -d only sets the debug level and -s only forces
> syslog. I guess this breakage does little harm as those flags are only
> used for dev (as they quits saned after first client).

I had some doubts about -s being "only used for dev" but upon reading
the manual page and the code, you're right.  The -s option behaves as
the -d option.  Their only difference is in where the output goes.

With that cleared up, I agree it is safe to say that no-one will be
using these options when running their saned service as a background
process.  If running saned on demand, e.g. via (x)inetd, you could use
them but I would frown upon doing so for "production" use.

# BTW, the saned manual page says you cannot give options when running
# from (x)inetd but that is incorrect or at least outdated.

Systemd?  No clue how that works and very little intent to find out.
If anyone in the know wants to chime in re running saned with these
options they're welcome.

# FWIW, I switched to Devuan in December 2016 after using Debian for
# about 19 years to regain init freedom ;-)  Alternative init system
# approaches are welcome!

Procd?  I'd think you know ;-)

> If not, I can introduce new flags for those actions and revert -d/-s
> previous behavior.

I don't think there is any need to keep these options backwardly
compatible but it might be in order for saned to warn people that these
options have changed since 1.0.27, show the new invocation for the old
behaviour and a pointer to the manual page for details on the changed
options.

Something similar could be done for -a, warning users that this option
is deprecated and will be removed in the future (without any explicit
mention of when exactly).

You mention something about creating a PID file for the -u option.  That
made me think a -p option to specify where you want that file might be a
nice addition.  The current location, /var/run/saned.pid, is hard-coded.
It's not a bad location but one may want to change it.

Oh, about the code changes, there are a few places in the manual page
I'd change to improve the English but I can do that for you.  There is
one mistake though, you document a -B option (as if it were -D).

I'd drop the SANED_EXEC_* defines you add because they're not used and
apart from a minor English nitpick in the option descriptions, the
saned.c changes look fine.

Hope this helps,
--
Olaf Meeuwissen, LPIC-2            FSF Associate Member since 2004-01-27
 GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13  F43E B8A4 A88A F84A 2DD9
 Support Free Software                        https://my.fsf.org/donate
 Join the Free Software Foundation              https://my.fsf.org/join