[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