>>- if (!std_info)
>>+ if (!std_info->stdid)
>> return -1;
>>
> This is a NACK. We shouldn't check for stdid since the function is supposed
> to update std_info. So just remove
>
> if (!std_info)
> return -1;
I don't see how std_info could get updated. consider the loop in case
std_info->stdid equals 0:
for (index = 0; index < ARRAY_SIZE(ch_params); index++) {
config = &ch_params[index];
(config is a local variable)
if (config->stdid & std_info->stdid) {
This fails for every index if std_info->stdid equals 0.
memcpy(std_info, config, sizeof(*config));
break;
}
}
So we always reach this with index == ARRAY_SIZE(ch_params)
if (index == ARRAY_SIZE(ch_params))
return -1;
So we could have returned -1 earlier.
> I am okay with the below change. So please re-submit the patch with the
> above change and my ACK.
>
> Thanks
>
> Murali
>
>>+ if (k == VPIF_DISPLAY_MAX_DEVICES)
>>+ k = VPIF_DISPLAY_MAX_DEVICES - 1;
actually I think this is still not right. shouldn't it be be
k = VPIF_DISPLAY_MAX_DEVICES - 1;
> are you using this driver in your project?
No, I just found this in the code.
Thanks, Roel
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
