MIME-Version: 1.0
References: <20230222220435.10688-1-palmer@rivosinc.com>
In-Reply-To: <20230222220435.10688-1-palmer@rivosinc.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 23 Feb 2023 13:43:41 +0200
Message-ID: <CAHp75Vd+TyxF87sdfaM=zwM_Jg_+qBJQP_BYK5t6u=bMd+EqYA@mail.gmail.com>
Subject: Re: [PATCH] lib/test_string.c: Add strncmp() tests
To:     Palmer Dabbelt <palmer@rivosinc.com>
Cc:     andy@kernel.org, linux-kernel@vger.kernel.org, heiko@sntech.de
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 23, 2023 at 12:10 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> The RISC-V strncmp() fails on some inputs, see the linked thread for
> more details.  It turns out there were no strncmp() calls in the self
> tests, this adds one.  It currently fails on RISC-V systems with Zbb
> enabled with
>
>     [    0.683479] String selftest failure 7.00001001

...

> This reports a checkpatch error for the __initconst, but I think it's
> spurious as I've just pattern matched the above test.

Does moving outside of function help?

...

> +               unsigned long count;
> +               unsigned long max_off;
> +               size_t retval;
> +       } tests[] __initconst = {
> +               { "/dev/vda", "/dev/",    5, 4, 0 },
> +               { "/dev/vda", "/dev/vdb", 5, 4, 0 },

In the current state the max_off is redundant.

> +               for (off = 0; off <= s->max_off; ++off) {

Why pre-increment?

...

> +                       if (res == 0 && s->retval != 0)
> +                               return 0x1000 + 0x100*off + 0x10*i + 0x0;
> +                       if (res > 0 && s->retval <= 0)
> +                               return 0x1000 + 0x100*off + 0x10*i + 0x1;
> +                       if (res < 0 && s->retval >= 0)
> +                               return 0x1000 + 0x100*off + 0x10*i + 0x2;

But you don't have cases for +1 -1 results (positive, negative), why
not add them?

-- 
With Best Regards,
Andy Shevchenko