linux-kernel - Re: [PATCH v2 03/35] mmap locking API: name the return values

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220131161737.cclyx2ntil3opted@revolver>
Date:   Mon, 31 Jan 2022 16:17:43 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Michel Lespinasse <michel@...pinasse.org>
CC:     Linux-MM <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "kernel-team@...com" <kernel-team@...com>,
        Laurent Dufour <ldufour@...ux.ibm.com>,
        Jerome Glisse <jglisse@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Michal Hocko <mhocko@...e.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Davidlohr Bueso <dave@...olabs.net>,
        Matthew Wilcox <willy@...radead.org>,
        Rik van Riel <riel@...riel.com>,
        Paul McKenney <paulmck@...nel.org>,
        Song Liu <songliubraving@...com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Minchan Kim <minchan@...gle.com>,
        Joel Fernandes <joelaf@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v2 03/35] mmap locking API: name the return values

* Michel Lespinasse <michel@...pinasse.org> [220128 08:10]:
> In the mmap locking API, the *_killable() functions return an error
> (or 0 on success), and the *_trylock() functions return a boolean
> (true on success).
> 
> Rename the return values "int error" and "bool ok", respectively,
> rather than using "ret" for both cases which I find less readable.

Would it be better to add function documentation in regards to return
types?  I think changing the variables does help, but putting a block
with Return: <what's above> would work best.

> 
> Signed-off-by: Michel Lespinasse <michel@...pinasse.org>
> ---
>  include/linux/mmap_lock.h | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index db9785e11274..1b14468183d7 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -81,22 +81,22 @@ static inline void mmap_write_lock_nested(struct mm_struct *mm, int subclass)
>  
>  static inline int mmap_write_lock_killable(struct mm_struct *mm)
>  {
> -	int ret;
> +	int error;
>  
>  	__mmap_lock_trace_start_locking(mm, true);
> -	ret = down_write_killable(&mm->mmap_lock);
> -	__mmap_lock_trace_acquire_returned(mm, true, ret == 0);
> -	return ret;
> +	error = down_write_killable(&mm->mmap_lock);
> +	__mmap_lock_trace_acquire_returned(mm, true, !error);
> +	return error;
>  }
>  
>  static inline bool mmap_write_trylock(struct mm_struct *mm)
>  {
> -	bool ret;
> +	bool ok;
>  
>  	__mmap_lock_trace_start_locking(mm, true);
> -	ret = down_write_trylock(&mm->mmap_lock) != 0;
> -	__mmap_lock_trace_acquire_returned(mm, true, ret);
> -	return ret;
> +	ok = down_write_trylock(&mm->mmap_lock) != 0;
> +	__mmap_lock_trace_acquire_returned(mm, true, ok);
> +	return ok;
>  }
>  
>  static inline void mmap_write_unlock(struct mm_struct *mm)
> @@ -120,22 +120,22 @@ static inline void mmap_read_lock(struct mm_struct *mm)
>  
>  static inline int mmap_read_lock_killable(struct mm_struct *mm)
>  {
> -	int ret;
> +	int error;
>  
>  	__mmap_lock_trace_start_locking(mm, false);
> -	ret = down_read_killable(&mm->mmap_lock);
> -	__mmap_lock_trace_acquire_returned(mm, false, ret == 0);
> -	return ret;
> +	error = down_read_killable(&mm->mmap_lock);
> +	__mmap_lock_trace_acquire_returned(mm, false, !error);
> +	return error;
>  }
>  
>  static inline bool mmap_read_trylock(struct mm_struct *mm)
>  {
> -	bool ret;
> +	bool ok;
>  
>  	__mmap_lock_trace_start_locking(mm, false);
> -	ret = down_read_trylock(&mm->mmap_lock) != 0;
> -	__mmap_lock_trace_acquire_returned(mm, false, ret);
> -	return ret;
> +	ok = down_read_trylock(&mm->mmap_lock) != 0;
> +	__mmap_lock_trace_acquire_returned(mm, false, ok);
> +	return ok;
>  }
>  
>  static inline void mmap_read_unlock(struct mm_struct *mm)
> -- 
> 2.20.1
>