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

Luiz Angelo Daros de Luca luizluca at gmail.com
Sun Sep 10 22:55:34 UTC 2017


Hi Olaf,

Apologies for the late follow-up.  Shame on me for pinging you on the
> bug report for no follow-up for a long time and then ignoring it when
> you promptly send some.  :-(
>
>
No apologies needed. We know how it works.

I was just waiting for your reply. My changes were already at github (and
just rebased):
https://github.com/luizluca/sane-backends/tree/reorganize-saned-args


> >> 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
> >
> > I beg to disagree. It's not true for (x)inet, systemd nor procd. They do
> > expect process in foreground. Only running saned standalone might need it
> > to go background. Also, I think that changing the default behavior is
> even
> > more traumatic than changing any behavior of options.
>
> Rethinking this, I guess you're right.  It's not much effort to manually
> background a daemon, simply add an `&` at the end of the command-line.
>

An `&` is not enough as there are some extra logic when going backgroud.
`-D` do just that.

> Something for the Release Notes as Changelog is built from git log. Is
> > there any place to save relevant changes for the next release?
>
> Eh, I guess the NEWS file would be as good a place as any.  Something
> like
>
>   New with the development version, not yet released:
>
> @Allan> Does that look okay?
>
>
I'll just wait for the "NEWS" answered you asked @Allan in order to update
only that and send patches to ML.

>> 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.
> >
> > I'll take a look. Normally PID file location is something for a
> > configuration file.
> > Sometimes init would like to take care of the PID file life cycle.
> Besides
> > being hard-coded, if a previous PID file exists, saned should do some
> > checks, and abort on failure. Today, if someone manage to run two
> instances
> > of a stand-alone saned, the last one would simply overwrite its own PID
> > inside the PID file. Also it should replace PID file instead of simply
> > rewriting it. At least it would avoid different code paths (and
> permission
> > requirements) whether a file at PID file exists or not.
>
> Looks like any user with write access to the directory holding the PID
> file can clobber it because
>
>   pidfile = fopen (SANED_PID_FILE, "w");
>
> doesn't pass O_EXCL to the underlying open().
>
> On my devuan box, /var/run is a symlink to /run which has 755 perms and
> is owned by root.  Digging further, /run is actually a mount point for a
> tmpfs.  But anyway, not all systems are created equally.
>

I'll leave PID_FILE as is for now.


> >> 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).
> >
> > Yeah, english skill aren't really my "expertise". :-) I do know my
> > limitations. Feel free to point me or correct them directly.
>
> I'll correct them directly (if there are any ;-).  It's less overhead
> for both of us.
>

OK for me.


> PS: I'll be travelling a bit in the next two weeks so will probably be
> late again in following up.
>

Nobody is at a hurry.

Regards,


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

Luiz Angelo Daros de Luca
luizluca at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20170910/1474b9af/attachment.html>


More information about the sane-devel mailing list