Re: [PATCH] mm, vmpressure: Fix a signedness bug in vmpressure_register_event()

From: Andy Shevchenko
Date: Wed Sep 25 2019 - 09:07:37 EST


On Wed, Sep 25, 2019 at 02:04:49PM +0300, Dan Carpenter wrote:
> The "mode" and "level" variables are enums and in this context GCC will
> treat them as unsigned ints so the error handling is never triggered.
>
> I also removed the bogus initializer because it isn't required any more
> and it's sort of confusing.
>

Indeed, assembly code is different.
Thank you for catching this!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Fixes: 3cadfa2b9497 ("mm/vmpressure.c: convert to use match_string() helper")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> mm/vmpressure.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> index f3b50811497a..fe077500cf20 100644
> --- a/mm/vmpressure.c
> +++ b/mm/vmpressure.c
> @@ -362,7 +362,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
> struct vmpressure *vmpr = memcg_to_vmpressure(memcg);
> struct vmpressure_event *ev;
> enum vmpressure_modes mode = VMPRESSURE_NO_PASSTHROUGH;
> - enum vmpressure_levels level = -1;
> + enum vmpressure_levels level;
> char *spec, *spec_orig;
> char *token;
> int ret = 0;
> @@ -376,7 +376,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
> /* Find required level */
> token = strsep(&spec, ",");
> level = match_string(vmpressure_str_levels, VMPRESSURE_NUM_LEVELS, token);
> - if (level < 0) {
> + if ((int)level < 0) {
> ret = level;
> goto out;
> }
> @@ -385,7 +385,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
> token = strsep(&spec, ",");
> if (token) {
> mode = match_string(vmpressure_str_modes, VMPRESSURE_NUM_MODES, token);
> - if (mode < 0) {
> + if ((int)mode < 0) {
> ret = mode;
> goto out;
> }
> --
> 2.20.1
>

--
With Best Regards,
Andy Shevchenko