Re: [PATCH v2 1/2] bitmap: Modify bitmap_set_value() to check bitmap length

From: Andy Shevchenko
Date: Sun Dec 13 2020 - 09:57:16 EST


On Sun, Dec 13, 2020 at 4:24 PM Syed Nayyar Waris <syednwaris@xxxxxxxxx> wrote:
>
> Add explicit check to see if the value being written into the bitmap
> does not fall outside the bitmap.
> The situation that it is falling outside would never be possible in the
> code because the boundaries are required to be correct before the
> function is called. The responsibility is on the caller for ensuring the
> boundaries are correct.
> The code change is simply to silence the GCC warning messages
> because GCC is not aware that the boundaries have already been checked.
> As such, we're better off using __builtin_unreachable() here because we
> can avoid the latency of the conditional check entirely.
>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Signed-off-by: Syed Nayyar Waris <syednwaris@xxxxxxxxx>
> Acked-by: William Breathitt Gray <vilhelm.gray@xxxxxxxxx>
>
> lib/test_bitmap.c: Modify for_each_set_clump test
>
> Modify the test where bitmap_set_value() is called. bitmap_set_value()
> now takes an extra bitmap-width as second argument and the width of
> value is now present as the fourth argument.
>
> Signed-off-by: Syed Nayyar Waris <syednwaris@xxxxxxxxx>
>
> gpio: xilinx: Modify bitmap_set_value() calls
>
> Modify the bitmap_set_value() calls. bitmap_set_value()
> now takes an extra bitmap width as second argument and the width of
> value is now present as the fourth argument.
>
> Cc: Michal Simek <michal.simek@xxxxxxxxxx>
> Signed-off-by: Syed Nayyar Waris <syednwaris@xxxxxxxxx>

Commit message here definitely needs more work.
First of all, it's now quite awkward to see this squashed stuff like this.
Second, it misses the warning examples it's talking about.
Third, it repeats some tags.
Fourth, it misses the Fixes tag.

Please, redone it correctly (one commit message with clear purpose and
example of warnings followed by Fixes tag) and resend a v3.

You may mention in the cover letter that this is squashed of three
patches from v1 (and give a link to lore.kernel.org).

--
With Best Regards,
Andy Shevchenko