[Dnsmasq-discuss] [PATCH] Do not crash when Ubus connection fails.
simon at thekelleys.org.uk
Sun Jun 27 20:56:05 UTC 2021
On 27/06/2021 20:19, Etan Kissling wrote:
> On 27.06.21, 19:48, "Simon Kelley" <simon at thekelleys.org.uk> wrote:
>> My change made the ubus code work in the same way as DBus. It expects
>> that ubus_init() will return a non-NULL error report if something
>> unexpected and nasty happened. (maybe a configuration that can never
>> work.) If the Ubus connection cannot be made, but that's expected to
>> change then ubus_init() should return NULL, and leave daemon->ubus set
>> to NULL. In that case ubus_init() will be called again, and can either
>> succeed, leave daemon->ubus still as NULL (in which case it will be
>> called again and again....) or return a fatal error, which can by now
>> only be logged. ubus_init() will continue to be called each time through
>> the event loop.
> Thanks for the really detailed explanations behind the change. It seems
> to me that basically, trying to run multiple dnsmasq instances with the
> same Ubus is not something that ever was supposed to work. Technically,
> as some of the instances quit, a different one could take over the Ubus
> connection, but this smells like "doing it incorrectly" in many ways.
> I will try to get the embedding project fixed to no longer try register
> multiple dnsmasq instances with the same Ubus instance name. This does
> not seem to have been right to begin with, entering "undefined behavior"
> territory, and now with the change it seems said "undefined behavior"
> decided to change. It makes totally sense to me to pick consistency with
> DBus over trying to preserve bug compatibility with client projects, so
> I no longer see a reason that this patch should be applied to dnsmasq.
I've committed 8a1ef367e27e570cac40d3b09920a4a60c5f7e0b which has the
same effect as your patch, but modifies the ubus code, and contains a
note that this needs to be looked at by someone who knows. It
more-or-less restores the status-quo ante, which helps the immediate
problem. In looking at the the long term fix, please submit or cause to
be submitted, better patches if they exist.
It might be worth pointing out that you can change the instance name
if that helps.
More information about the Dnsmasq-discuss