linux-kernel - Re: [PATCHv3 1/2 RESEND] doc/media api: Try to make enum usage clearer

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]

Date:   Wed, 15 Mar 2023 14:09:14 +0100
From:   Dorota Czaplejewicz <dorota.czaplejewicz@...i.sm>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...i.sm
Subject: Re: [PATCHv3 1/2 RESEND] doc/media api: Try to make enum usage
 clearer

Hi,

On Wed, 15 Mar 2023 12:14:28 +0200
Sakari Ailus <sakari.ailus@....fi> wrote:

> Hi Dorota,
> 
> On Sun, Feb 19, 2023 at 12:00:34PM +0100, Dorota Czaplejewicz wrote:
> > This clarifies which side of the calls is responsible for doing what to which parts of the struct.
> > This also explicitly states that repeating values are disallowed.
> > It also expands the terse description of the access algorithm into more prose-like, active voice description, which trades conciseness for ease of comprehension.  
> 
> The commit message lines should be wrapped at 74 characters. Please do use
> checkpatch.pl in the future. There is also no apparent reason to add a
> newline after a period.
> 
> The same applies to the other patch as well.
> 
> I've addressed the issues this time.

Thanks for taking a look. What is the next step?

--Dorota
> 
> > 
> > Added: mbus codes must not repeat
> > Added: no holes in the enumeration
> > Added: enumerations per what?
> > Added: who fills in what in calls
> > Changed: "zero" -> "0"
> > Changed: "given" -> "specified"
> > 
> > Still unclear how it works so didn't describe: "which". What is a "try format" vs "active format"?
> > 
> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@...i.sm>
> > ---
> > Hi,
> > 
> > I took those patches out of the freezer. I made sure they still apply (they do), and corrected some errors pointed out by Jacopo in the previous round of reviews.  
> 
> Ditto.
> 
> > 
> > Thanks,
> > Dorota
> > 
> >  .../v4l/vidioc-subdev-enum-mbus-code.rst      | 38 +++++++++++++------
> >  1 file changed, 26 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > index 417f1a19bcc4..63bff24d0520 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > @@ -31,15 +31,28 @@ Arguments
> >  Description
> >  ===========
> >  
> > -To enumerate media bus formats available at a given sub-device pad
> > -applications initialize the ``pad``, ``which`` and ``index`` fields of
> > -struct
> > -:c:type:`v4l2_subdev_mbus_code_enum` and
> > -call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl with a pointer to this
> > -structure. Drivers fill the rest of the structure or return an ``EINVAL``
> > -error code if either the ``pad`` or ``index`` are invalid. All media bus
> > -formats are enumerable by beginning at index zero and incrementing by
> > -one until ``EINVAL`` is returned.
> > +This call is used by the application to access the enumeration
> > +of media bus formats for the selected pad.
> > +
> > +The enumerations are defined by the driver, and indexed using the ``index`` field
> > +of struct :c:type:`v4l2_subdev_mbus_code_enum`.
> > +Each enumeration starts with the ``index`` of 0, and
> > +the lowest invalid index marks the end of enumeration.
> > +
> > +Therefore, to enumerate media bus formats available at a given sub-device pad,
> > +initialize the ``pad``, and ``which`` fields to desired values,
> > +and set ``index`` to 0.
> > +Then call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl
> > +with a pointer to this structure.
> > +
> > +A successful call will return with the ``code`` field filled in
> > +with a mbus code value.
> > +Repeat with increasing ``index`` until ``EINVAL`` is received.
> > +``EINVAL`` means that either ``pad`` is invalid,
> > +or that there are no more codes available at this pad.
> > +
> > +The driver must not return the same value of ``code`` for different indices
> > +at the same pad.
> >  
> >  Available media bus formats may depend on the current 'try' formats at
> >  other pads of the sub-device, as well as on the current active links.
> > @@ -57,14 +70,15 @@ information about the try formats.
> >  
> >      * - __u32
> >        - ``pad``
> > -      - Pad number as reported by the media controller API.
> > +      - Pad number as reported by the media controller API. Filled in by the application.  
> 
> Here, too...
> 
> >      * - __u32
> >        - ``index``
> > -      - Number of the format in the enumeration, set by the application.
> > +      - Index of the mbus code in the enumeration belonging to the given pad.
> > +    Filled in by the application.  
> 
> Wrong indentation.
> 
> >      * - __u32
> >        - ``code``
> >        - The media bus format code, as defined in
> > -	:ref:`v4l2-mbus-format`.
> > +	:ref:`v4l2-mbus-format`. Filled in by the driver.
> >      * - __u32
> >        - ``which``
> >        - Media bus format codes to be enumerated, from enum  
> 


Content of type "application/pgp-signature" skipped