Hi,
Le dimanche 12 mai 2013 à 11:34 +0200, Pablo Neira Ayuso a écrit :
> Hi Chris,
>
> On Sun, May 12, 2013 at 09:11:51AM +0100, Chris Boot wrote:
> > On 12/05/2013 01:48, Pablo Neira Ayuso wrote:
> > > On Sat, May 11, 2013 at 09:27:31PM +0100, Chris Boot wrote:
> > > [...]
> > >> Hi Pablo,
> > >>
> > >> I'd argue exactly the opposite point: that when you want multiple
> > >> instances a PID file can help you work out which is which.
> > > That new option may break existing setups with multiple instances.
> >
> > My patch explicitly doesn't change the behaviour of existing
> > configurations. If you don't pass '--pidfile /path/to/file.pid', no pid
> > file is written and there is no change in how ulogd works.
>
> Existing setups having already two ulogd2 instances will break, as
> they won't be passing --pidfile, thus clashing on the same default pid
> file. One of the instances will not proceed. They will have to add
> --pidfile to their scripts to get things back working.
If I read the patch correctly, the pidfile is not created if the option
is not given:
In main we have:
+ if (write_pidfile() < 0)
+ warn_and_exit(daemonize);
and in the write_pidfile() function we have:
+static int write_pidfile()
+{
+ int pid_fp;
+ char pidtext[16];
+ int len;
+
+ if (!ulogd_pidfile)
+ return 0;
So, the default behavior is not changed.
If the default behavior is not changed, I see no problem in adding the
pidfile feature. And if it can help to get ulogd into distributions, I'm
in favor of it ;)
But, as pointed out by Pablo's reading of the code, testing if we need
to write the file only inside of write_pidfile() is a bit confusing
something like:
if (ulogd_pidfile) write_pidfile(); // add error handling here
would be better.
BR,
--
Eric
Attachment:
signature.asc
Description: This is a digitally signed message part
