Re: [PATCH v2] mm: zsmalloc: Replace return type int with bool

[Michal Hocko <mhocko@kernel.org>]

On Wed 21-02-18 16:48:50, Souptick Joarder wrote:
> On Wed, Feb 21, 2018 at 3:52 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Tue 20-02-18 23:28:11, Souptick Joarder wrote:
> > [...]
> >> -static int zs_register_migration(struct zs_pool *pool)
> >> +static bool zs_register_migration(struct zs_pool *pool)
> >>  {
> >>       pool->inode = alloc_anon_inode(zsmalloc_mnt->mnt_sb);
> >>       if (IS_ERR(pool->inode)) {
> >>               pool->inode = NULL;
> >> -             return 1;
> >> +             return true;
> >>       }
> >>
> >>       pool->inode->i_mapping->private_data = pool;
> >>       pool->inode->i_mapping->a_ops = &zsmalloc_aops;
> >> -     return 0;
> >> +     return false;
> >>  }
> >
> > Don't you find it a bit strange that the function returns false on
> > success?
> 
> The original code was returning 0 on success  and return value was handled
> accordingly in zs_create_pool(). So returning false on success.

Returning 0 on success and an error code on failure is a standard
convention. Returning false is just weird. zs_register_migration is
somewhere in the middle because it doesn't really return an error code
on failure. Whether this is worth bothering and fixing is a question for
the maintainer but returning false on success is just not an improvement
IMHO.

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>