[PATCH 2/2] bearer-mbim: parse replies on failure

Bjørn Mork bjorn at mork.no
Thu Dec 19 23:35:31 PST 2013


Aleksander Morgado <aleksander at lanedo.com> writes:

> On 19/12/13 18:29, Aleksander Morgado wrote:
>>>      if (response &&
>>> > -        mbim_message_command_done_get_result (response, &error) &&
>>> > +        (mbim_message_command_done_get_result (response, &error) ||
>>> > +         error->code == MBIM_STATUS_ERROR_FAILURE) &&
>>> >          mbim_message_connect_response_parse (
>>> >              response,
>>> >              &session_id,
>>> > @@ -440,6 +441,7 @@ connect_set_ready (MbimDevice *device,
>>> >              &nw_error,
>>> >              &error)) {
>>> >          if (nw_error) {
>>> > +            g_error_free (error);
>>> >              error = mm_mobile_equipment_error_from_mbim_nw_error (nw_error);
>>> >              mm_dbg ("NwError: '%s' (%u)", error->message, nw_error);
>>> >          } else {
>> I'll rewrite a bit that logic, because there may be the case where we
>> get an error in mbim_message_command_done_get_result() with
>> MBIM_STATUS_ERROR_FAILURE, and *also* afterwards a
>> mbim_message_connect_response_parse() error. Which would lead to a leak
>> in the first GError we got.
>
> How about this? (attached)

Looks good to me.  I'll give a bit of testing.

But wouldn't you have saved yourself a bit of worrying if all functions
potentinally allocating new errors, like mbim_message_connect_response_parse,
also included something like:

 if (*error)
    g_error_free (*error);

?


Bjørn


More information about the ModemManager-devel mailing list