| Message ID | 20240829152025.3203577-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] docs: netdev: document guidance on cleanup.h | expand |
On Thu, Aug 29, 2024 at 5:20 PM Jakub Kicinski <kuba@kernel.org> wrote: > > Document what was discussed multiple times on list and various > virtual / in-person conversations. guard() being okay in functions > <= 20 LoC is my own invention. If the function is trivial it should > be fine, but feel free to disagree :) > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> +2 Reviewed-by: Eric Dumazet <edumazet@google.com>
On 29/08/2024 18:20, Jakub Kicinski wrote: > Document what was discussed multiple times on list and various > virtual / in-person conversations. guard() being okay in functions > <= 20 LoC is my own invention. If the function is trivial it should > be fine, but feel free to disagree :) > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: andrew@lunn.ch > CC: corbet@lwn.net > CC: linux-doc@vger.kernel.org > --- > Documentation/process/maintainer-netdev.rst | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst > index fe8616397d63..ccd6c96a169b 100644 > --- a/Documentation/process/maintainer-netdev.rst > +++ b/Documentation/process/maintainer-netdev.rst > @@ -392,6 +392,22 @@ When working in existing code which uses nonstandard formatting make > your code follow the most recent guidelines, so that eventually all code > in the domain of netdev is in the preferred format. > > +Using device-managed and cleanup.h constructs > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Netdev remains skeptical about promises of all "auto-cleanup" APIs, > +including even ``devm_`` helpers, historically. They are not the preferred > +style of implementation, merely an acceptable one. > + > +Use of ``guard()`` is discouraged within any function longer than 20 lines, > +``scoped_guard()`` is considered more readable. Using normal lock/unlock is > +still (weakly) preferred. > + > +Low level cleanup constructs (such as ``__free()``) can be used when building > +APIs and helpers, especially scoped interators. However, direct use of > +``__free()`` within networking core and drivers is discouraged. > +Similar guidance applies to declaring variables mid-function. > + > Resending after review > ~~~~~~~~~~~~~~~~~~~~~~ > Definitely and strongly agree. Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
On Thu, Aug 29, 2024 at 08:20:25AM -0700, Jakub Kicinski wrote: > Document what was discussed multiple times on list and various > virtual / in-person conversations. guard() being okay in functions > <= 20 LoC is my own invention. If the function is trivial it should > be fine, but feel free to disagree :) > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> When discussing guard() etc, i have said we would reconsider this in a couple of years looking at the experience of other subsystems. Maybe something about this could be added to the commit message? Otherwise: Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Thu, Aug 29, 2024 at 08:20:25AM -0700, Jakub Kicinski wrote: > Document what was discussed multiple times on list and various > virtual / in-person conversations. guard() being okay in functions > <= 20 LoC is my own invention. If the function is trivial it should > be fine, but feel free to disagree :) > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: andrew@lunn.ch > CC: corbet@lwn.net > CC: linux-doc@vger.kernel.org > --- > Documentation/process/maintainer-netdev.rst | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst > index fe8616397d63..ccd6c96a169b 100644 > --- a/Documentation/process/maintainer-netdev.rst > +++ b/Documentation/process/maintainer-netdev.rst > @@ -392,6 +392,22 @@ When working in existing code which uses nonstandard formatting make > your code follow the most recent guidelines, so that eventually all code > in the domain of netdev is in the preferred format. > > +Using device-managed and cleanup.h constructs > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Netdev remains skeptical about promises of all "auto-cleanup" APIs, > +including even ``devm_`` helpers, historically. They are not the preferred > +style of implementation, merely an acceptable one. > + > +Use of ``guard()`` is discouraged within any function longer than 20 lines, > +``scoped_guard()`` is considered more readable. Using normal lock/unlock is > +still (weakly) preferred. > + > +Low level cleanup constructs (such as ``__free()``) can be used when building > +APIs and helpers, especially scoped interators. However, direct use of Sorry to nit-pick: iterators > +``__free()`` within networking core and drivers is discouraged. > +Similar guidance applies to declaring variables mid-function. > + > Resending after review > ~~~~~~~~~~~~~~~~~~~~~~ > > -- > 2.46.0 > >
diff --git a/Documentation/process/maintainer-netdev.rst b/Documentation/process/maintainer-netdev.rst index fe8616397d63..ccd6c96a169b 100644 --- a/Documentation/process/maintainer-netdev.rst +++ b/Documentation/process/maintainer-netdev.rst @@ -392,6 +392,22 @@ When working in existing code which uses nonstandard formatting make your code follow the most recent guidelines, so that eventually all code in the domain of netdev is in the preferred format. +Using device-managed and cleanup.h constructs +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Netdev remains skeptical about promises of all "auto-cleanup" APIs, +including even ``devm_`` helpers, historically. They are not the preferred +style of implementation, merely an acceptable one. + +Use of ``guard()`` is discouraged within any function longer than 20 lines, +``scoped_guard()`` is considered more readable. Using normal lock/unlock is +still (weakly) preferred. + +Low level cleanup constructs (such as ``__free()``) can be used when building +APIs and helpers, especially scoped interators. However, direct use of +``__free()`` within networking core and drivers is discouraged. +Similar guidance applies to declaring variables mid-function. + Resending after review ~~~~~~~~~~~~~~~~~~~~~~
Document what was discussed multiple times on list and various virtual / in-person conversations. guard() being okay in functions <= 20 LoC is my own invention. If the function is trivial it should be fine, but feel free to disagree :) Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: andrew@lunn.ch CC: corbet@lwn.net CC: linux-doc@vger.kernel.org --- Documentation/process/maintainer-netdev.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)