linux-kernel - Re: [RFCv4 11/21] media: v4l2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPBb6MVAKKjStJoaa7QJJc+Ay7kgYRbYsgchf5Sr+HrY7rOAsA@mail.gmail.com>
Date:   Wed, 21 Feb 2018 15:01:36 +0900
From:   Alexandre Courbot <acourbot@...omium.org>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Pawel Osciak <posciak@...omium.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Tomasz Figa <tfiga@...omium.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Gustavo Padovan <gustavo.padovan@...labora.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFCv4 11/21] media: v4l2_fh: add request entity field

On Wed, Feb 21, 2018 at 12:24 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> On 02/20/18 05:44, Alexandre Courbot wrote:
>> Allow drivers to assign a request entity to v4l2_fh. This will be useful
>> for request-aware ioctls to find out which request entity to use.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@...omium.org>
>> ---
>>  include/media/v4l2-fh.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
>> index ea73fef8bdc0..f54cb319dd64 100644
>> --- a/include/media/v4l2-fh.h
>> +++ b/include/media/v4l2-fh.h
>> @@ -28,6 +28,7 @@
>>
>>  struct video_device;
>>  struct v4l2_ctrl_handler;
>> +struct media_request_entity;
>>
>>  /**
>>   * struct v4l2_fh - Describes a V4L2 file handler
>> @@ -43,6 +44,7 @@ struct v4l2_ctrl_handler;
>>   * @navailable: number of available events at @available list
>>   * @sequence: event sequence number
>>   * @m2m_ctx: pointer to &struct v4l2_m2m_ctx
>> + * @entity: the request entity this fh operates on behalf of
>>   */
>>  struct v4l2_fh {
>>       struct list_head        list;
>> @@ -60,6 +62,7 @@ struct v4l2_fh {
>>  #if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
>>       struct v4l2_m2m_ctx     *m2m_ctx;
>>  #endif
>> +     struct media_request_entity *entity;
>
> The name 'media_request_entity' is very confusing.
>
> In the media controller API terminology an entity represents a piece
> of hardware with inputs and outputs (very rough description), but a
> request is not an entity. It may be associated with an entity, though.
>
> So calling this field 'entity' is also very misleading.

Note that this field is not a request though, it is a pointer to a
piece of hardware referenced by a request, which is closer to the MC
terminology. Or do you mean this should just be renamed
"request_entity"?

If we go all the way, the media_ prefix is also misleading - it
implies a dependency to the media controller framework, while there is
none (in this patchset at least).

However I thought that 'request' alone (instead of media_request) may
name-conflict with something else, and since 'media' is also the
umbrella term for anything under drivers/media it sounds fitting on
the other hand. Suggestions are welcome though.

>
> As with previous patches, I'll have to think about this and try and
> come up with better, less confusing names.

I will gladly take suggestions, have been trying to come with a better
name to reply to your comment above but could not find any. :)