On Tue, Apr 7, 2020 at 5:20 PM Andrzej Ostruszka <a...@semihalf.com> wrote:
>
> 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.
All DPDK _dump() functions returns void. I thought, We will keep the same here.
Another option is. if it is NULL we can return.
i.e
-RTE_ASSERT(f != NULL);
+if (f == NULL)
+ return
Either scheme is OK with me, Let me know your preference, I will
change accordingly.
>
> But I'm fine if you ignore this.
>
> With regards
> Andrzej Ostruszka
