Re: [PATCH v4 2/7] soc: qcom: qmi: add a way to remove running service

[Chris Lew <quic_clew@quicinc.com>]

On 3/13/2024 5:09 PM, Dmitry Baryshkov wrote:
> On Thu, 14 Mar 2024 at 02:03, Chris Lew <quic_clew@quicinc.com> wrote:
>>
>>
>>
>> On 3/11/2024 8:34 AM, Dmitry Baryshkov wrote:
>>> +/**
>>> + * qmi_del_server() - register a service with the name service
>>
>> s/register/deregister/g
> 
> ack
> 
>>
>>> + * @qmi:     qmi handle
>>> + * @service: type of the service
>>> + * @instance:        instance of the service
>>> + * @version: version of the service
>>> + *
>>> + * Remove registration of the service with the name service. This notifies
>>> + * clients that they should no longer send messages to the client associated
>>> + * with @qmi.
>>> + *
>>> + * Return: 0 on success, negative errno on failure.
>>> + */
>>> +int qmi_del_server(struct qmi_handle *qmi, unsigned int service,
>>> +                unsigned int version, unsigned int instance)
>>> +{
>>> +     struct qmi_service *svc;
>>> +     struct qmi_service *tmp;
>>> +
>>> +     list_for_each_entry_safe(svc, tmp, &qmi->services, list_node) {
>>> +             if (svc->service != service ||
>>> +                 svc->version != version ||
>>> +                 svc->instance != instance)
>>> +                     continue;
>>> +
>>> +             qmi_send_del_server(qmi, svc);
>>> +             list_del(&svc->list_node);
>>> +             kfree(svc);
>>> +
>>> +             return 0;
>>> +     }
>>> +
>>
>> is list_for_each_entry_safe required if we stop iterating and return
>> after we find the first instance of the service?
> 
> Yes, it just adds a temp variable here.
> 

Ok, I was thinking that tmp wasn't necessary because we don't continue
iterating through the list after we free the svc. I guess it never hurts
to be safe though.

>>
>>> +     return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(qmi_del_server);
> 
> 
>