[dpdk-dev] [PATCH v7 2/6] bus: introduce parsing functionality

[santosh]

On Thursday 06 July 2017 06:00 PM, Gaëtan Rivet wrote:

> On Thu, Jul 06, 2017 at 02:49:41PM +0530, santosh wrote:
>> Hi Gaetan,
>>
>> On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
>>
>>> This operation can be used either to validate that a device
>>> representation can be understood by a bus, as well as store the resulting
>>> specialized device representation in any format determined by the bus.
>>>
>>> Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
>>> ---
>>>  lib/librte_eal/common/include/rte_bus.h | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
>>> index 773b0d7..aebf57e 100644
>>> --- a/lib/librte_eal/common/include/rte_bus.h
>>> +++ b/lib/librte_eal/common/include/rte_bus.h
>>> @@ -138,6 +138,26 @@ typedef int (*rte_bus_plug_t)(struct rte_device *dev,
>>>  typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
>>>  
>>>  /**
>>> + * Bus specific parsing function.
>>> + * Validates the syntax used in the textual representation of a device,
>>> + * If the syntax is valid and ``addr`` is not NULL, writes the bus-specific
>>> + * device representation to ``addr``.
>>> + *
>>> + * @param[in] name
>>> + *	device textual description
>>> + *
>>> + * @param[out] addr
>>> + *	device information location address, into which parsed info
>>> + *	should be written. If NULL, nothing should be written, which
>>> + *	is not an error.
>>> + *
>> r / is not a error/ is valid?
>>
> I'm partial to "is not an error" here, but it doesn't matter that much
> and I can change it if you prefer.
>
>>> + * @return
>>> + *	0 if parsing was successful.
>>> + *	!0 for any error.
>>> + */
>>> +typedef int (*rte_bus_parse_t)(const char *name, void *addr);
>>> +
>> _parse handler in _common_vdev or common_pci file return boolean value 
>> i.e..0 for success and 1 for error, right? if so then
>> !0 for any error would be like '1' for error case.. make sense?
>>
> I thought of that yes, and actually your suggestion was the first
> version I used.
>
> Ultimately however, this function is not only saying "can parse": it is
> not merely a test of being able to process the input, but also the
> process itself. The test value is then a byproduct.
>
> As such, I decided to settle on the standard "0 means nothing of note
> happened, carry on".

I'm not aware of past work history, catching up with stuff so no
strong opinion but In my opinion: if we're sure about return values then better
mention them explicitly.

Thanks.