Re: [dpdk-dev] [PATCH 1/8] vhost: add security model documentation to vhost_user.c

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi John,

On 02/07/2018 05:10 PM, Mcnamara, John wrote:
> 
> 
>> -----Original Message-----
>> From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
>> Sent: Tuesday, February 6, 2018 2:23 PM
>> To: Kovacevic, Marko <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Yuanhan
>> Liu <yliu@fridaylinux.org>; Mcnamara, John <john.mcnamara@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 1/8] vhost: add security model
>> documentation to vhost_user.c
>>
>> On Tue, Feb 06, 2018 at 01:26:13PM +0000, Kovacevic, Marko wrote:
>>>> Input validation is not applied consistently in vhost_user.c.  This
>>>> suggests that not everyone has the same security model in mind when
>>>> working on the code.
>>>>
>>>> Make the security model explicit so that everyone can understand and
>>>> follow the same model when modifying the code.
>>>>
>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>> ---
>>>>   lib/librte_vhost/vhost_user.c | 17 +++++++++++++++++
>>>>   1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/lib/librte_vhost/vhost_user.c
>>>> b/lib/librte_vhost/vhost_user.c
>>>
>>> <...>
>>>
>>> This is a useful comment but I don't know if it makes sense to include
>> it in the vhost_user.c file.
>>>
>>> Particularly at the top where it looks like a general descriptive
>> comment for the file.
>>>
>>> It would probably be better in the vhost-user section of the
>> programmer's guide:
>>>
>>> http://dpdk.org/doc/guides/prog_guide/vhost_lib.html
>>
>> That is the public API documentation for users of the library.  They don't
>> parse VhostUserMsg so I'm not sure the comment would be relevant there.
> 
> Hi,
> 
> If it is public API documentation then it should probably be in a .h file
> in doxygen format.
> 
> I'm in favour of the information being added, and thanks for that, just not
> in that position in that file.

This is not part of the API but purely vhost-user lib internals,
so I think this is the right place for this comment.

It is more likely to be seen by the developer here than in a separate
file.

Cheers,
Maxime

> John
>