Re: [PATCH V3 2/2] debugfs: don't assume sizeof(bool) to be 4 bytes

Charles Keepax
2015-09-15T09:13:19Z

On Tue, Sep 15, 2015 at 02:04:59PM +0530, Viresh Kumar wrote:
> Long back 'bool' type used to be a typecast to 'int', but that changed
> in v2.6.19. And that is a typecast to _Bool now, which (mostly) takes
> just a byte. Anyway, the bool type is implementation defined, and better
> we don't assume its size to be 4 bytes or 1.
>
> The problem with current code is that it reads/writes 4 bytes for a
> boolean, which will read/update 3 excess bytes following the boolean
> variable (when sizeof(bool) is 1 byte). And that can lead to hard to fix
> bugs. It was a nightmare cracking this one.
>
> The debugfs code had this bug since the first time it got introduced,
> but was never got caught, strange. Maybe the bool variables (monitored
> by debugfs) were followed by an 'int' or something bigger and the pad
> bytes made sure, we never see this issue.
>
> But the OPP (Operating performance points) library have three booleans
> allocated to contiguous bytes and this bug got hit quite soon (The
> debugfs support for OPP is yet to be merged). It showed up as corruption
> of the debugfs boolean symbols, where Y were becoming N and vice versa.
>
> Fix it properly by changing the last argument of debugfs_create_bool(),
> to type 'bool *' instead of 'u32 *', so that it doesn't depend on sizeof
> bool at all.
>
> That required updates to all user sites as well in a single commit.
> regmap core was also using debugfs_{read|write}_file_bool(), directly
> and variable types were updated for that to be bool as well.
>
> Acked-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

For the minor wm_adsp change:

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles