linux-kernel - Re: [PATCH] docs: checkpatch: Document and segregate more checkpatch message types

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] [day] [month] [year] [list]

Message-ID: <eab0487d7b4e68badbbe0505b2a7903b9d8931c4.camel@perches.com>
Date:   Fri, 04 Jun 2021 23:26:35 -0700
From:   Joe Perches <joe@...ches.com>
To:     Dwaipayan Ray <dwaipayanray1@...il.com>, corbet@....net,
        lukas.bulwahn@...il.com
Cc:     linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] docs: checkpatch: Document and segregate more
 checkpatch message types

On Sat, 2021-06-05 at 11:29 +0530, Dwaipayan Ray wrote:
> Add and document more checkpatch message types. About 50% of all
> message types are documented now.
[]
> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
[]
> +  **DEVICE_ATTR_FUNCTIONS**
> +    The function names used in DEVICE_ATTR is unusual.
> +    Typically, the store and show functions are named as name_store and
> +    name_show, where name is the device name.

No, it's the variable name of an attribute of a device, not the device name.

    Typically, the store and show functions are used with <attr>_store and
    <attr>_show, where <attr> is a named attribute variable of the device.

> +    Consider the following examples::
> +
> +      static DEVICE_ATTR(type, 0444, type_show, NULL);
> +      static DEVICE_ATTR(power, 0644, power_show, power_store);
> +
> +    The function names should preferably follow the above pattern.
> +
> +    See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
> +
> +  **DEVICE_ATTR_RO**
> +    The DEVICE_ATTR_RO(name) helper macro can be used in place of
> +    DEVICE_ATTR(name, 0444, name_show, NULL);
> +
> +    Note that the macro automatically appends _show to the device name
> +    for the show method.

attribute, etc...

> +  **ENOSYS**
> +    ENOSYS means that a nonexistent system call was called.  We have a
> +    bad habit of using it for things like invalid operations on
> +    otherwise valid syscalls.  This should be avoided in new code.

Please do not use terms like "we".  Just use passive voice and not
any first person/collective words.

> +
> +    See: https://lore.kernel.org/lkml/5eb299021dec23c1a48fa7d9f2c8b794e967766d.1408730669.git.luto@amacapital.net/
> +
> +  **ENOTSUPP**
> +    ENOTSUPP is not a standard error code and should be avoided in new patches.
> +    EOPNOTSUPP should be used instead.

Better word choice is like this section above.