On 4/7/20 12:22 PM, Jerin Jacob wrote:
> On Mon, Apr 6, 2020 at 11:47 PM Andrzej Ostruszka <a...@semihalf.com> wrote:
>>
>> On 4/5/20 10:55 AM, jer...@marvell.com wrote:
>>> From: Jerin Jacob <jer...@marvell.com>
>>>
>>> Adding node debug API implementation support to dump
>>> single or all the node objects to the given file.
>>>
>>> Signed-off-by: Jerin Jacob <jer...@marvell.com>
>>> Signed-off-by: Kiran Kumar K <kirankum...@marvell.com>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
>>> Signed-off-by: Nithin Dabilpuram <ndabilpu...@marvell.com>
>> [...]
>>> diff --git a/lib/librte_graph/node.c b/lib/librte_graph/node.c
>>> index d04a0fce0..8592c1221 100644
>>> --- a/lib/librte_graph/node.c
>>> +++ b/lib/librte_graph/node.c
>>> @@ -377,6 +377,38 @@ rte_node_edge_get(rte_node_t id, char *next_nodes[])
>>>       return rc;
>>>  }
>>>
>>> +static void
>>> +node_scan_dump(FILE *f, rte_node_t id, bool all)
>>> +{
>>> +     struct node *node;
>>> +
>>> +     RTE_ASSERT(f != NULL);
>>
>> Why the assert?  Below this is used in public (I guess) functions so
>> user can provide wrong input - in that case I'd expect warning/error not
>> an assert.
> 
> Public API rte_node_dump() and node_scan_dump() calls this API without
> any check.

That was my point.  I would expect either there or here to have a check
for arg instead of assert.  I'd say that asserts are very good for
checking internal logic, but not so for checking if user input is OK.

But I'm fine if you ignore this.

With regards
Andrzej Ostruszka

Reply via email to