[ovs-dev] [PATCH v10 4/5] dpif-netdev: refactor generic implementation

Ilya Maximets i.maximets at samsung.com
Fri Jul 12 16:33:16 UTC 2019


On 12.07.2019 19:20, Van Haaren, Harry wrote:
>>> +    subtable->mf_masks = xmalloc(sizeof(uint64_t) * (unit0 + unit1));
>>> +    netdev_flow_key_gen_masks(mask, subtable->mf_masks, unit0, unit1);
>>> +
>>> +    /* Allocate blocks scratch space only if subtable requires more size
>> than
>>> +     * is currently allocated. */
>>> +    const uint32_t blocks_required_per_pkt = unit0 + unit1;
>>> +    if (cls->blocks_scratch_size < blocks_required_per_pkt) {
>>> +        free(cls->blocks_scratch);
>>> +        cls->blocks_scratch = xmalloc(sizeof(uint64_t) * NETDEV_MAX_BURST *
>>> +                                      blocks_required_per_pkt);
>>
>> For sizeof above I don't think you need the () as it's part of an
>> expression. See OVS code standard below. This may apply to the xmalloc
>> for mf_masks above as well.
>>
>> The sizeof operator is unique among C operators in that it accepts two
>> very different kinds of operands: an expression or a type. In general,
>> prefer to specify an expression, e.g. int *x = xmalloc(sizeof *x);. When
>> the operand of sizeof is an expression, there is no need to parenthesize
>> that operand, and please don't.
> 
> Hmmm, I get compile failures here with "sizeof uint64_t" instead of "sizeof(uint64_t)".
> There's also some ambiguity possible, see here: https://stackoverflow.com/a/26702432
> 
> In short, I've left the sizeof() with brackets, as code that compiles wins in the end.

It's suggested to use sizeof without brackets but for the pointer variable.
So, in OVS coding style this should look like:

       cls->blocks_scratch = xmalloc(NETDEV_MAX_BURST
                                     * blocks_required_per_pkt
                                     * sizeof *cls->blocks_scratch);

For mf_masks:

   subtable->mf_masks = xmalloc((unit0 + unit1) * sizeof *subtable->mf_masks);

Best regards, Ilya Maximets.


More information about the dev mailing list