[dpdk-dev] [PATCH v3 01/29] graph: define the public API for graph support

Andrzej Ostruszka amo at semihalf.com
Mon Apr 6 18:09:36 CEST 2020


On 4/6/20 4:59 PM, Jerin Jacob wrote:
> On Mon, Apr 6, 2020 at 6:06 PM Andrzej Ostruszka <amo at semihalf.com> wrote:
[...]
>>> +typedef uint32_t rte_graph_off_t;  /**< Graph offset type. */
>>> +typedef uint32_t rte_node_t;       /**< Node id type. */
>>> +typedef uint16_t rte_edge_t;       /**< Edge id type. */
>>> +typedef uint16_t rte_graph_t;      /**< Graph id type. */
>>
>> I would use 'id' somewhere in the name of these typedefs - e.g. seeing
>> rte_node_t in the code (without knowing what it is) I'd be guessing this
>> is a pointer to 'struct rte_node'.
>> So maybe 'rte_node_id' or if we stick with _t convention and
>> rte_node_id_t is too long then maybe simple rte_nid_t/rte_eid_t/rte_gid_t?
> 
> Considering typedef will not be pointers in Linux coding standard, I
> have chosen shorter
> name. considering eid, gid is cryptic and Since you think, rte_node_id
> better, I will change to
> that.

If the typedef are not pointers by the coding standard then I'm fine
with current names - no need to change.

[...]
>> [...]
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice
>>> + *
>>> + * Get the number of edges for a node from node id.
>>> + *
>>> + * @param id
>>> + *   Valid node id.
>>> + *
>>> + * @return
>>> + *   Valid edge count on success, RTE_EDGE_ID_INVALID otherwise.
>>> + */
>>> +__rte_experimental
>>> +rte_edge_t rte_node_edge_count(rte_node_t id);
>>
>> I would clarify here what edge is?  Incoming nodes, next-nodes or both.
> 
> It is next-node. I will update the doc.
> 
>>  Why edge-id typedef on return and why EDGE_ID_INVALID returned.  Would
>> int/-EINVAL (for wrong 'id') be better?
> 
> Edge node ID is expressed in rte_edge_id_t. SO, I think, it fine to return
> rte_edge_id_t. "This would avoid any compassion issue as well."

Did not understand the last sentence.  Could you rephrase it?

With regards
Andrzej Ostruszka


More information about the dev mailing list