Thread (22 messages) 22 messages, 6 authors, 2021-02-03

Re: [PATCH v3] ext4: Enable code path when DX_DEBUG is set

From: Vinicius Tinti <hidden>
Date: 2021-02-03 09:52:58
Also in: lkml

On Wed, Feb 3, 2021 at 2:39 AM Theodore Ts'o [off-list ref] wrote:
On Tue, Feb 02, 2021 at 04:28:37PM +0000, Vinicius Tinti wrote:
quoted
Clang with -Wunreachable-code-aggressive is being used to try to find
unreachable code that could cause potential bugs. There is no plan to
enable it by default.

The following code was detected as unreachable:

fs/ext4/namei.c:831:17: warning: code will never be executed
[-Wunreachable-code]
                        unsigned n = count - 1;
                                     ^~~~~
fs/ext4/namei.c:830:7: note: silence by adding parentheses to mark code as
explicitly dead
                if (0) { // linear search cross check
                    ^
                    /* DISABLES CODE */ ( )

This has been present since commit ac27a0ec112a ("[PATCH] ext4: initial
copy of files from ext3") and fs/ext3 had it present at the beginning of
git history. It has not been changed since.

This patch moves the code to a new function `htree_rep_invariant_check`
which only performs the check when DX_DEBUG is set. This allows the
function to be used in other parts of the code.

Suggestions from: Andreas, Christoph, Nathan, Nick and Ted.

Signed-off-by: Vinicius Tinti <redacted>
Thanks, applied, although I rewrote the commit description:

    ext4: factor out htree rep invariant check

    This patch moves some debugging code which is used to validate the
    hash tree node when doing a binary search of an htree node into a
    separate function, which is disabled by default (since it is only used
    by developers when they are modifying the htree code paths).

    In addition to cleaning up the code to make it more maintainable, it
    silences a Clang compiler warning when -Wunreachable-code-aggressive
    is enabled.  (There is no plan to enable this warning by default, since
    there it has far too many false positives; nevertheless, this commit
    reduces one of the many false positives by one.)

The original description buried the lede, in terms of the primary
reason why I think the change was worthwhile (although I know you have
different priorities than mine :-).

Thanks for working to find a way to improve the code in a way that
makes both of us happy!
Thanks for the feedback.

And thanks for all the ones who reviewed.
                                        - Ted
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help