On 11/17/2016 04:30 PM, Will Deacon wrote:
> On Thu, Nov 10, 2016 at 04:15:12PM +0100, G. Campana wrote:
>> On 08/11/2016 03:08, Will Deacon wrote:
>>> On Tue, Oct 18, 2016 at 06:02:52PM +0200, G. Campana wrote:
>>>> ---
>>>> virtio/9p.c | 60 +++++++++++++++++++++++++++++++++++++++++++-----------------
>>>> 1 file changed, 43 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/virtio/9p.c b/virtio/9p.c
>>>> index 9695540..cd93d06 100644
>>>> --- a/virtio/9p.c
>>>> +++ b/virtio/9p.c
>>>> @@ -28,6 +28,7 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid)
>>>> {
>>>> struct rb_node *node = dev->fids.rb_node;
>>>> struct p9_fid *pfid = NULL;
>>>> + size_t len;
>>>>
>>>> while (node) {
>>>> struct p9_fid *cur = rb_entry(node, struct p9_fid, node);
>>>> @@ -45,9 +46,15 @@ static struct p9_fid *find_or_create_fid(struct p9_dev *dev, u32 fid)
>>>> if (!pfid)
>>>> return NULL;
>>>>
>>>> + len = strlen(dev->root_dir);
>>>> + if (len >= sizeof(pfid->path)) {
>>>> + free(pfid);
>>>> + return NULL;
>>>> + }
>>>
>>> This doesn't make sense to me -- pfid->path is just a NULL ptr at this
>>> stage. Did you mean abs_path?
>>>
>> Good catch. Indeed, I meant abs_path.
>>
>>>> +
>>>> pfid->fid = fid;
>>>> - strcpy(pfid->abs_path, dev->root_dir);
>>>> - pfid->path = pfid->abs_path + strlen(dev->root_dir);
>>>> + strncpy(pfid->abs_path, dev->root_dir, sizeof(pfid->abs_path));
>>>
>>> But if you did mean abs_path, why use strncpy here?
>>>
>> It ensures that the string is null terminated.
>
> Given that we already did a strlen on root_dir, and we know that abs_path
> is big enough to hold it, I still think this isn't needed.
>
Even if we know that abs_path is big enough, I prefer strncpy over
strcpy because it won't mislead static analysis tools and the overhead
is negligible. Anyway, it doesn't matter in the end, and I can submit a
new patch if you want.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
