On Wed, Apr 14, 2021 at 6:34 AM Ian Abbott <abbotti@xxxxxxxxx> wrote: > > On 14/04/2021 11:09, Dan Carpenter wrote: > > The test_ni_get_reg_value() function calls > > unittest(ni_get_reg_value_roffs(-1, O(0), T, 1) == -1, > > "check bad direct trigger arg for first reg->dest w/offs\n"); > > The -1 is type promoted to a high positive value so src is greater than > > NI_NAMES_BASE. It's not clear that that was intentional. > > drivers/staging/comedi/drivers/tests/../ni_routes.h:269 ni_get_reg_value_roffs() warn: 'src' possible negative type promoted to high > > I agree that it appears that ni_get_reg_value_roffs() will be returning > -1 as expected, but for the wrong reason. I think the intention was for > ni_get_reg_value_roffs() to call route_register_is_valid() with src=0 in > this case, but it actually calls ni_route_to_register() with src=-1 > (because -1 gets converted to 0xffffffff in the test `if (src < > NI_NAMES_BASE)` where NI_NAMES_BASE is defined as 0x8000u). > Good catch. Based on the error message that for the unittest failure, I would agree that my intention had been to test when src is indeed < NI_NAMES_BASE so that we could test for a bad direct register value. It does indeed look like sending in 0 for the src would have worked, since the first row in private table RV in ni_routes_test.c (the row for destination passed in as "O(0)") does not have a "register value" equal to 0. It would be nice to compile and test this code though, but I can't do it for another couple of weeks. Actually, looking a little further, I don't think that src=0 will work here. There is another argument for ni_get_reg_value_roffs, direct_reg_offset, that gets added to src before sending to route_register_is_valid. I think this was provided to fix an offset mismatch that was in the original approach to handling signal routing values (i.e. to provide backwards compatibility). But, I do think that value greater or equal to 9 will actually work.
