Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

From: Stanimir Varbanov
Date: Fri May 05 2017 - 09:29:13 EST


Hi Bjorn

On 05/02/2017 09:52 PM, Bjorn Andersson wrote:
> On Tue 02 May 01:52 PDT 2017, Stanimir Varbanov wrote:
>
>> Hei Sakari,
>>
>> On 04/30/2017 01:21 AM, Sakari Ailus wrote:
>>> Hi, Stan!!
>>>
>>> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
>>> ...
>>>> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
>>>> + struct hfi_buffer_requirements *req)
>>>> +{
>>>> + u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
>>>> + union hfi_get_property hprop;
>>>> + int ret, i;
>>>
>>> unsigned int i ? It's an array index...
>>
>> Thanks for pointing that out, I have to revisit all similar places as
>> well ...
>>
>
> It's perfectly fine to index an array with an int and you are comparing
> the index with a integer constant in the loop - so don't clutter the
> code unnecessarily.

I personally prefer unsigned for iterator variable type (because
unsigned type has defined behavior on overflow), but having the fact
that I'm comparing with int I will keep it int.

Also it seems that -Wsign-compare is not enabled by default in kernel,
no? So I have modified my Makefile and catch few occurrences of warnings
about signed with unsigned compare and fixed them.

--
regards,
Stan