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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
