On Aug 8, 2012, at 3:23 AM, [email protected] wrote:
> From: Patrik Flykt <[email protected]>
> Subject: Re: [PATCH v2] service: signal error property changes
> Date: August 8, 2012 3:15:39 AM PDT
> To: connman <[email protected]>
> Reply-To: [email protected]
> 
> Noticed one issue, and some nitpicks.
> 
> On Mon, 2012-08-06 at 09:28 -0700, Grant Erickson wrote:
>> 
>> static void set_idle(struct connman_service *service)
>> {
>>      service->state = service->state_ipv4 = service->state_ipv6 =
>>                                              CONNMAN_SERVICE_STATE_IDLE;
>> -    service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +    set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>>      state_changed(service);
>> }
>> 
>> @@ -4100,6 +4125,8 @@ static void service_initialize(struct connman_service 
>> *service)
>>      service->refcount = 1;
>>      service->session_usage_count = 0;
>> 
>> +    service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +
>>      service->type     = CONNMAN_SERVICE_TYPE_UNKNOWN;
>>      service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
>> 
>> @@ -4679,7 +4706,7 @@ static void request_input_cb (struct connman_service 
>> *service,
>>  done:
>>      if (err >= 0) {
>>              /* We forget any previous error. */
>> -            service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +            set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>> 
>>              __connman_service_connect(service);
> 
> It'd be better to call set_error() in __connman_service_connect() since
> it clears the error in all of the service connect cases, e.g on
> autoconnect, VPNs, Session API use, etc. This one I missed last time,
> sorry for that.

Patrik,

Thanks for the feedback and review. Regarding the last comment, I'd prefer 
addressing that in a second, orthogonal patch as that does more than just 
replace existing assignments with the function call.

What do you think?

-Grant
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to