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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 30 Jul 2025 11:31:35 +0200
Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@xxxxxxxxxx/
> 
> 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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux