Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test

[roel kluin]

>>- Â Â Â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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/