Re: [dpdk-dev] [PATCH v5 3/3] ipfrag: add unit test case

[Aaron Conole <aconole@redhat.com>]

"Burakov, Anatoly" <anatoly.burakov@intel.com> writes:

> On 17-Apr-20 2:14 PM, Aaron Conole wrote:
>> Initial IP fragmentation unit test.
>>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>
> <snip>
>
>> +	if (!pkt_pool)
>> +		pkt_pool = rte_pktmbuf_pool_create("FRAG_MBUF_POOL",
>> +						   NUM_MBUFS, BURST, 0,
>> +						   RTE_MBUF_DEFAULT_BUF_SIZE,
>> +						   SOCKET_ID_ANY);
>> +	if (pkt_pool == NULL) {
>> +		printf("%s: Error creating pkt mempool\n", __func__);
>> +		goto bad_setup;
>> +	}
>> +
>> +	if (!direct_pool)
>> +		direct_pool = rte_pktmbuf_pool_create("FRAG_D_MBUF_POOL",
>> +						      NUM_MBUFS, BURST, 0,
>> +						      RTE_MBUF_DEFAULT_BUF_SIZE,
>> +						      SOCKET_ID_ANY);
>> +	if (!direct_pool) {
>> +		printf("%s: Error creating direct mempool\n", __func__);
>> +		goto bad_setup;
>> +	}
>> +
>> +	if (!indirect_pool)
>> +		indirect_pool = rte_pktmbuf_pool_create("FRAG_I_MBUF_POOL",
>> +							NUM_MBUFS, BURST, 0,
>> +							0, SOCKET_ID_ANY);
>> +	if (!indirect_pool) {
>> +		printf("%s: Error creating indirect mempool\n", __func__);
>> +		goto bad_setup;
>> +	}
>
> Nitpicking, but i believe the coding style guide discourages using
> boolean syntax for anything other than boolean checks, and it is
> better to use a more explicit `if (x == NULL)`.

I see, it does.  Looking at the code-base, I see it mixed all over, some
places using 'if (!ptr)' and others 'if (ptr == NULL)'. Actually, even
in the flow_filtering.rst doc, it implies that if (!ptr) is acceptable.

Since I'm spinning a v6 with the constants, I'll fold this change in -
maybe it makes sense to clean it up everywhere to help mitigate the
confusion (for example, I most recently did work in the eal and the !ptr
is all over there).  WDYT?