hidpp20 driver error handling in libratbag

[Peter Hutterer]

On Wed, Nov 21, 2018 at 06:45:31PM +0100, Haochen Tong wrote:
> Hi,
> 
> First I would like to say thanks for libratbag.
> 
> I'm trying to add support for MX Anywhere 2 but I get a crash by just
> changing the device IDs in the MX Master 2 device file. The crash is
> reported here[1].
> 
> While digging into the crash I am confused about the handling of return
> values. In the hidpp20 driver there is a call chain like:
> 
> (1) hidpp20drv_init_feature
> (2)   -> hidpp20drv_read_special_key_mouse
> (3)     -> hidpp20_special_key_mouse_get_controls
> (4)       -> hidpp20_special_keys_buttons_get_reporting
> (5)         -> hidpp20_request_command
> (6)           -> hidpp20_request_command_allow_error
> 
> In (1) it seems rc=0 means OK, and negative value shows an error.
> In (2) rc is also used as num_controls when positive.
> But in (6) the HID++ protocol error number, which is a positive integer
> according to document[2], is returned, and it gets propagated all the
> way up to (2), which then causes problem, because (2) expects rc to be a
> valid num_controls when positive, not an HID++ protocol error. This
> problem later causes the aforementioned crash, because
> drv_data->controls is 0 while drv_data->num_controls is positive.
> 
> I'm thinking about fixing it but not really sure how to deal with both
> protocol and system errors in a sane way.
> 
> In addition, in the function hidpp20_special_key_mouse_get_controls, if
> get_info or get_reporting fails for any special key, all other keys are
> ignored (free(c_list)). Is this resonable? On my mouse I always get an
> "invalid argument" error when trying to get the reporting method of the
> 7th key.
> 
> Thanks in advance for any guidance.
> 
> [1] https://github.com/libratbag/libratbag/issues/558
> [2] https://drive.google.com/file/d/0BxbRzx7vEV7eU3VfMnRuRXktZ3M/view

For the archives: we've investigated this on the issue linked above and it's
been fixed. It was basically an error code confusion. Please see that link
for details.

Cheers,
   Peter