On 4 August 2016 at 02:30, Maxim Uvarov <maxim.uva...@linaro.org> wrote:

> On 08/03/16 22:21, Anders Roxell wrote:
>
>> On 2 August 2016 at 17:27, Mike Holmes <mike.hol...@linaro.org> wrote:
>>
>>> On 2 August 2016 at 04:44, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
>>>
>>> On 08/02/16 10:54, Anders Roxell wrote:
>>>>
>>>> Signed-off-by: Anders Roxell <anders.rox...@linaro.org>
>>>>> ---
>>>>>    helper/hashtable.c | 6 +++---
>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/helper/hashtable.c b/helper/hashtable.c
>>>>> index 8bb1ae5..f17b80f 100644
>>>>> --- a/helper/hashtable.c
>>>>> +++ b/helper/hashtable.c
>>>>> @@ -164,7 +164,7 @@ odph_table_t odph_hash_table_lookup(const char
>>>>> *name)
>>>>>     * This hash algorithm is the most simple one, so we choose it as an
>>>>> DEMO
>>>>>     * User can use any other algorithm, like CRC...
>>>>>     */
>>>>> -uint16_t odp_key_hash(void *key, uint32_t key_size)
>>>>> +static uint16_t odp_key_hash(void *key, uint32_t key_size)
>>>>>
>>>>> odph_
>>>>
>>>>    {
>>>>>          register uint32_t hash = 0;
>>>>>          uint32_t idx = (key_size == 0 ? 1 : key_size);
>>>>> @@ -181,7 +181,7 @@ uint16_t odp_key_hash(void *key, uint32_t key_size)
>>>>>    /**
>>>>>     * Get an available node from pool
>>>>>     */
>>>>> -odph_hash_node *odp_hashnode_take(odph_table_t table)
>>>>> +static odph_hash_node *odp_hashnode_take(odph_table_t table)
>>>>>    {
>>>>>          odph_hash_table_imp *tbl = (odph_hash_table_imp *)table;
>>>>>          uint32_t idx;
>>>>> @@ -208,7 +208,7 @@ odph_hash_node *odp_hashnode_take(odph_table_t
>>>>> table)
>>>>>    /**
>>>>>     * Release an node to the pool
>>>>>     */
>>>>> -void odp_hashnode_give(odph_table_t table, odph_hash_node *node)
>>>>> +static void odp_hashnode_give(odph_table_t table, odph_hash_node
>>>>> *node)
>>>>>
>>>>>
>>>>> odph_
>>>>
>>>
>>> Should this patch change the name as well ?
>>>
>> I would say no. =)
>>
>> The fix appears to be to correctly make internal functions static whist
>>> converting to a naming scheme is a separate fix
>>>
>> agree.
>>
>> Cheers,
>> Anders
>>
>
> I don't mind if it will be 2 patches or 1 patch. If new issue was observed
> during review it has to be fixed with the same patch set.
> Or if you don't want/have time then you can file bug and we will take care
> about how will fix it.
>
> I would like to see one single patch here because you should not fix
> something partially and introduce patch lines with errors. If you
> touched that line than in my opinion it's logical to fix problem correct
> and completely. Problem from patch description is "to hide private
> functions".
>

Some times I agree that makes sense, but it is better to make a bug for a
second issue if  a patch clearly fixes the issues it claims to, without a
balance we won't make progress if every patch requires fixing  other issues
that are not directly implicated.



>
> regards,
> Maxim,
>
>
>
>>> {
>>>>
>>>>>          odph_hash_table_imp *tbl = (odph_hash_table_imp *)table;
>>>>>
>>>>>
>>>>>
>>>>
>>> --
>>> Mike Holmes
>>> Technical Manager - Linaro Networking Group
>>> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM
>>> SoCs
>>> "Work should be fun and collaborative, the rest follows"
>>>
>>
>


-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"

Reply via email to