Re: [PATCH 08/15] service: Leverage 'service_set_proxy_method'.

Hi Grant,

On 2/21/25 9:56 AM, Grant Erickson wrote:
> Leverage 'service_set_proxy_method' for
> 'connman_service_set_proxy_method', the first of two call sites.

Could this commit description be clearer?  E.g. mention 
connman_service_set_proxy_method() instead of 'first of two call sites'?

> ---
>   src/service.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index e58958635992..5c94c2962de8 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -5617,19 +5617,14 @@ static void service_set_proxy_method(struct connman_service *service,
>   void connman_service_set_proxy_method(struct connman_service *service,
>   					enum connman_service_proxy_method method)
>   {
> -	DBG("service %p (%s) method %d (%s)",
> -		service, connman_service_get_identifier(service),
> -		method, proxymethod2string(method));
> -
> -	if (!service || service->hidden)
> -		return;
> -
> -	service->proxy = method;
> +	const bool donotifier = method != CONNMAN_SERVICE_PROXY_METHOD_AUTO;
> +	void * const context = NULL;

What's the point to this variable?

>   
> -	proxy_changed(service);
> -
> -	if (method != CONNMAN_SERVICE_PROXY_METHOD_AUTO)
> -		__connman_notifier_proxy_changed(service);
> +	service_set_proxy_method(service,
> +		method,
> +		donotifier,
> +		NULL,
> +		context);

Just use NULL here which is clearer?

>   }
>   
>   enum connman_service_proxy_method connman_service_get_proxy_method(

Regards,
-Denis