Re: [PATCH] service: Fix default service update on ready state

From: Daniel Wagner <wagi@monom.org>
To: "VAUTRIN Emmanuel (Canal Plus Prestataire)"
	<Emmanuel.VAUTRIN@cpexterne.org>
Cc: "connman@lists.linux.dev" <connman@lists.linux.dev>
Subject: Re: [PATCH] service: Fix default service update on ready state
Date: Fri, 2 Jul 2021 09:41:02 +0200	[thread overview]
Message-ID: <20210702074102.7rmohctvgqzzn6fb@beryllium.lan> (raw)
In-Reply-To: <20210702072703.ye5a33wi7jk6qbyf@beryllium.lan>

Hi Emmanuel,

On Fri, Jul 02, 2021 at 09:27:03AM +0200, Daniel Wagner wrote:
> > > This is needed on top of my reorder patch?
> > Indeed.
> 
> Okay, this one is easy to understand. Patch applied.

After looking at your other patches I dropped the patch.

By moving default_changed() after

	def_service = connman_service_get_default();
	service_update_preferred_order(def_service, service, new_state);

the result is the code doesn't make sense anymore. Why calling
service_update_preferred_order *before* we change the default?

This is the very reason I am pushing back against these changes. They
look innocent and might even work for some cases, but anyone later
looking at this code will scratch his head and will say "this doesn't
make sense". Also The commit message saying "the default shall be
re-evaluated" doesn't help either. You need to explain why you need to
change the order and why service_update_preferred_order() should operate
on the 'outdated' default route.

Thanks,
Daniel