[PATCH] mbimcli: check status code in command-done message before parsing response
bjorn at mork.no
Fri Feb 28 00:31:26 PST 2014
Aleksander Morgado <aleksander at aleksander.es> writes:
> On 27/02/14 19:23, Ben Chan wrote:
>> This patch adds missing mbim_message_command_done_get_result() checks in
>> mbimcli to ensure that the status code in a command-done message reports
>> no error before parsing further parsing the response. It also adds
>> missing mbim_message_unref() calls to ensure that the response is freed
>> in case of an error.
> Pushed, thanks.
Looks good to me. Tested with MC7710, DWM-156 and E367 respectively:
bjorn at nemi:/usr/local/src/git/libmbim$ src/mbimcli/mbimcli -d /dev/cdc-wdm0 --ms-query-firmware-id
[/dev/cdc-wdm0] Firmware ID retrieved: '00000000-0000-0000-0000-000000000000'
bjorn at nemi:/usr/local/src/git/libmbim$ src/mbimcli/mbimcli -d /dev/cdc-wdm1 --ms-query-firmware-id
error: operation failed: NoDeviceSupport
bjorn at nemi:/usr/local/src/git/libmbim$ src/mbimcli/mbimcli -d /dev/cdc-wdm2 --ms-query-firmware-id
error: operation failed: Transaction timed out
>> Hi Aleksander,
>> The error handling code seems quite repetitive. Perhaps we can later refactor
>> the common code into some helper function.
> Yeah, we could do that. But then, we still need to handle the
> non-standard cases where a status error code is given along with a valid
> information buffer (i.e. the helper function will be only for a subset
> of the cases).
The non-standard cases are few and well defined. You could push the list
of exceptions to the helper, maybe?
More information about the libmbim-devel