Sorry for the delay.
> I am not convinced that your patch is complete correct. In case
> __connman_service_connect() returns with success instead of EINPROGRESS
> then your patch could it up the response, no?
> Suppose that autoconnect is happening in the background and the user
> calls 'Connect' before the Service was online. In this case we used to
> send an 'error already' back.
> The question is why does ConnMan send the same response twice? Can you
> send a log?
I assumed that in function connect_service, upon return from
__connman_service_connect if service->pending is NULL, it means that a
reply has been sent. Depending on the code path taken from
__connman_service_connect, if there is a call to reply_pending, then the
first reply is sent and service->pending is sets to NULL.
Ah! I didn't see the reply_pending() inside __connman_service_connect().
That explains the double send.
Maybe I should make the patch more explicit, something like:
if service->pending is null then return null? Or do you think there are
places where service->pending can be set to NULL without sending a reply?
I think the problem is that we do have two different expectation on how
__connman_service_connect() should handle errors. In most cases
__connman_service_connect() just returns the error code. Currently only
service_connect() is checking the return value and sends then a reply in
the error case. Also __connman_service_disconnect() has this two kinds
of error handling.
Hmm, what about moving reply_pending() out of
__connman_service_connect() and __connman_service_disconnect() and send
the error message from the calling side? This would make it more
explicit what part of the code is in charge for sending messages. Note
we currently write 'reply_pending(service)' which already indicates
something is wrong