Jeroen van Wolffelaar wrote:
> [snip]
>>> Fine. I did put the function back where it was though, instead of moving
>>> it way below, because the stuff in between can hang (for example, when
>>> failing to connect, it'll indefinitely retry every 15 seconds, causing
>>> 'start' to hang).
>>>
>>>
>> Yup. I noticed that.
>> From my POV, it is related to a "misconfiguration" of the server ---
>> being configured to connect to a nonexistent/malfunctioning IMAP server.
>> However, I do realize it could hang the booting process, so it should be
>> fixed.
>>
>> ACK to your solution
>>
>
> It's a misconfiguration, sure, but one that happens on default install
> (out of the box imapproxy is misconfigured? Well, it was in my case, but
> probably related to the fact that I don't actually have any imap server
> running on my test machine :)
>
Well, I do. But I did test that case (pointing to a non-existant server)
Imapproxy's configuration process does ask you for a server, so a
misconfiguration there is certainly due to the user.
> In any case, yeah, the problem is that start should simply never hang.
>
>
>>> Moving Daemonize to later on was not related to this RC bug. Putting it
>>> in its own function isn't either, but is pretty harmless, so ok.
>>>
>>>
>> It does allow to move where Daemonize is called easily ;)
>>
>>>> - added support for the "-p pidfile" option
>>>>
>>>>
>>> Cool. There was a buglet here -- you didn't initialize the var and then
>>> only overwrote it with the default if it started with a nulbyte. I
>>> changed that to simply always initialize it to the default.
>>>
>>>
>> Indeed. I know better than to have a variable unitialized, BUT I
>> followed upstream's style here (the same pattern as with the configfile)
>> -- I am aiming for inclusion of this by now HUGE patch, anyway...
>>
>
> Yes, but in that case, you failed to properly copy&paste the full thing
> -- upstream *does* set the first byte of ConfigFile to \0, you didn't
> copy that bit.
:-\ Touché.
> You also didn't completely copy the logging to be
> analogous.
>
This was on purpose.
>>>> I have tested starting / restarting / stopping / re-stopping
>>>>
>>> Added code to not actually fail on second start / fail on second stop
>>> that I had already prepared. It now should really work fine in all
>>> cases.
>>>
>>> I also removed dead code from checking the exit state of a 'true'. I
>>> removed the "|| true" so that the script just fails right there when it
>>> eh, fails.
>>>
>>>
>> ACK. I will re-check the Developer's Reference for mandated/recommended
>> messages and initscript behaviour in those cases not covered by the Policy.
>>
>
> I'm not so sure either dev-ref or policy are very verbose on this, it'd
> be good for someone to look into this at some point... But world peace
> would also be nice :).
>
Ok. I will try and bug developers-reference for this :-)
> [snip]
> Anyway, thanks a lot for your work on the pidfile handling, it was
> convenient to only need to verify, and not really write it all.
>
Thank you, Jeroen.
It was a pleasure working with both of you (Thank you too, Steve)
J.L.
