[FFmpeg-devel] [PATCH] Set frame rate on v4l2 devices

[Måns Rullgård]

jose.goncalves at inov.pt writes:

> Quoting M?ns Rullg?rd <mans at mansr.com>:
>
>> No, coding style is things like placement of braces, which I did not
>> comment on.
>>
>>> I agree with most of them (not all). But... if you take a look to
>>> the v4l2_set_parameters() function you will see that I followed the
>>> coding style previously used, what seemed the correct thing to do.
>>
>> Old mistakes are no excuse for making new ones.
>
> New patch attempt attached. Some comments though.
>
> For me, initializing a variable in declaration or not, or placing a
> comment or not, is coding style... but let's not argue about that.
>
> I don't agree with two of your comments.
>
> First, initializing a struct with {0} instead of using a memset(). I
> always use memset() because gcc gives a warning of "missing
> initializer" in the other way if you activate the '-Wextra' option.

Don't do that then.  The C standard makes it quite clear that missing
elements in an initialiser are implicitly zero.  Just do it.

> Second, testing ioctl error against -1 instead of negative. As rule of
> thumb I test for a function error as the negation of returning
> OK. Most ioctls return 0 for OK, but some return a positive value,
> hence testing for negative.

The ioctl() definition says it returns -1 on error.  Therefore,
testing for -1 is correct and testing for negative is not.  The same
goes for most system calls.  This is a very common mistake but a
mistake nonetheless.

-- 
M?ns Rullg?rd
mans at mansr.com