linux-kernel - Re: [PATCH 3/4] agents: add coding style documentation and rules

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: <20250730111044.37146f6f@gandalf.local.home>
Date: Wed, 30 Jul 2025 11:10:44 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Sasha Levin <sashal@...nel.org>, Kees Cook <kees@...nel.org>,
 corbet@....net, linux-doc@...r.kernel.org, workflows@...r.kernel.org,
 josh@...htriplett.org, konstantin@...uxfoundation.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] agents: add coding style documentation and rules

On Wed, 30 Jul 2025 11:31:35 +0200
Krzysztof Kozlowski <krzk@...nel.org> wrote:

> I pop up there a lot, but there is no confusion. I am (and maybe we are
> all?) well aware that checkpatch hard limit is 100 as explained also here:
> https://lore.kernel.org/all/df2e466a-cdaa-4263-ae16-7bf56c0edf21@kernel.org/
> 
> But the coding style still says that preferred length limit is 80.
> Checkpatch is not a coding style. Coding style document is describing
> the coding style...
> 
> People trust checkpatch way too much, thus its hard limit was raised.
> Some maintainers also agree with that, yet it does not invalidate what
> coding style document says.

Yeah, I had a couple of patches that were sent to me with everything at 100
max (comments and all). As I still have my windows set to 80 columns by
default, I find it annoying. I told them to fix it and resubmit.

But a break here and there where it makes it look a little better doesn't
bother me. For instance, the code in kernel/trace/trace.c has:

        if (tif_need_resched())
                trace_flags |= TRACE_FLAG_NEED_RESCHED;
        if (test_preempt_need_resched())
                trace_flags |= TRACE_FLAG_PREEMPT_RESCHED;
        if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY))
                trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY;
        return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) |
                (min_t(unsigned int, migration_disable_value(), 0xf)) << 4;

Where

        if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY))

Breaks the 80 char limit, but honestly, I rather have that than:

        if (tif_need_resched())
                trace_flags |= TRACE_FLAG_NEED_RESCHED;
        if (test_preempt_need_resched())
                trace_flags |= TRACE_FLAG_PREEMPT_RESCHED;
        if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) &&
	    tif_test_bit(TIF_NEED_RESCHED_LAZY))
                trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY;
        return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) |
                (min_t(unsigned int, migration_disable_value(), 0xf)) << 4;

As that breaks the flow.

Thus, to me it's a guideline. Try to stay under 80 but we don't need to be
draconian about it.

-- Steve