linux-kernel - Re: [PATCH v3 00/30] staging: rtl8723bs: remove RT

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:   Sat, 03 Apr 2021 08:02:25 -0700
From:   Joe Perches <joe@...ches.com>
To:     Fabio Aiuto <fabioaiuto83@...il.com>, gregkh@...uxfoundation.org
Cc:     dan.carpenter@...cle.com, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 00/30] staging: rtl8723bs: remove RT_TRACE logs in
 core/*

On Sat, 2021-04-03 at 11:13 +0200, Fabio Aiuto wrote:
> This patchset removes all RT_TRACE usages in core/ files.

and hal and include and os_dep

> 
> This is the first of a series aimed at removing RT_TRACE macro.
> 
> The whole private tracing system is not tied to a configuration
> symbol and the default behaviour is _trace nothing_. It's verbose
> and relies on a private log level tracing doomed to be
> removed.

It's nice, but individual patches per file done by hand are difficult
to review because you are interleaving removal patches with cleanup
patches.

I believe this should be a patch series with a single patch to remove
all RT_TRACE macro uses using coccinelle and then use separate patches
to do whatever cleanups around these removals you want to do.

All of these below should be done for all files in drivers/staging/rtl8723bs
at once instead of submitting per-file patches.

IMO something like:

Cover-letter: Explain why you are doing this
Patch 1 of N: Remove all RT_TRACE macro uses using a coccinelle script
              and include the coccinelle script in the commit message
Patch 2 of N: Remove commented out RT_TRACE uses
Patch 3 of N: Remove RT_TRACE macro definition
Patch 4 of N: Cleanup coccinelle generated {} uses, if/else braces and
              the now unnecessary if tests around the RT_TRACE removals
Patch 5 of N: Cleanup whitespace
Patcn x of N: Whatever else related to these RT_TRACE sites...

https://lore.kernel.org/lkml/c845d8ea7d0d8e7a613471edb53d780d660142a9.camel@perches.com/

Using a sequence like the above would be much easier to review and
would be a significant shorter patch set.