linux-kernel - Re: [PATCH] linux/kernel.h: break long lines

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: <20180224183044.GB29374@pd.tnic>
Date:   Sat, 24 Feb 2018 19:30:44 +0100
From:   Borislav Petkov <bp@...e.de>
To:     Gustavo Leite <gustavoleite.ti@...il.com>
Cc:     linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Kees Cook <keescook@...omium.org>, Baoquan He <bhe@...hat.com>,
        Ian Abbott <abbotti@....co.uk>,
        Randy Dunlap <rdunlap@...radead.org>,
        Niklas Söderlund 
        <niklas.soderlund+renesas@...natech.se>
Subject: Re: [PATCH] linux/kernel.h: break long lines

On Sat, Feb 24, 2018 at 03:10:02PM -0300, Gustavo Leite wrote:
> Break lines that are longer than 80 characters do match the linux coding
> style.
> 
> Signed-off-by: Gustavo Leite <gustavoleite.ti@...il.com>
> ---
>  include/linux/kernel.h | 74 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 49 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index ce51455e2adf..13b84cfd09a9 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -307,10 +307,12 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
>  #endif
>  
>  /* Internal, do not use. */
> -int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
> +int __must_check _kstrtoul(const char *s, unsigned int base,
> +		unsigned long *res);
>  int __must_check _kstrtol(const char *s, unsigned int base, long *res);
>  

But this:

> -int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);

is more readable than this:

> +int __must_check kstrtoull(const char *s, unsigned int base,
> +		unsigned long long *res);

> -int __must_check kstrtoull_from_user(const char __user *s, size_t count, unsigned int base, unsigned long long *res);
> -int __must_check kstrtoll_from_user(const char __user *s, size_t count, unsigned int base, long long *res);
> -int __must_check kstrtoul_from_user(const char __user *s, size_t count, unsigned int base, unsigned long *res);
> -int __must_check kstrtol_from_user(const char __user *s, size_t count, unsigned int base, long *res);
> -int __must_check kstrtouint_from_user(const char __user *s, size_t count, unsigned int base, unsigned int *res);
> -int __must_check kstrtoint_from_user(const char __user *s, size_t count, unsigned int base, int *res);
> -int __must_check kstrtou16_from_user(const char __user *s, size_t count, unsigned int base, u16 *res);
> -int __must_check kstrtos16_from_user(const char __user *s, size_t count, unsigned int base, s16 *res);
> -int __must_check kstrtou8_from_user(const char __user *s, size_t count, unsigned int base, u8 *res);
> -int __must_check kstrtos8_from_user(const char __user *s, size_t count, unsigned int base, s8 *res);
> -int __must_check kstrtobool_from_user(const char __user *s, size_t count, bool *res);
> -
> -static inline int __must_check kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)
>
> +int __must_check kstrtoull_from_user(const char __user *s, size_t count,
> +		unsigned int base, unsigned long long *res);
> +int __must_check kstrtoll_from_user(const char __user *s, size_t count,
> +		unsigned int base, long long *res);
> +int __must_check kstrtoul_from_user(const char __user *s, size_t count,
> +		unsigned int base, unsigned long *res);
> +int __must_check kstrtol_from_user(const char __user *s, size_t count,
> +		unsigned int base, long *res);
> +int __must_check kstrtouint_from_user(const char __user *s, size_t count,
> +		unsigned int base, unsigned int *res);
> +int __must_check kstrtoint_from_user(const char __user *s, size_t count,
> +		unsigned int base, int *res);
> +int __must_check kstrtou16_from_user(const char __user *s, size_t count,
> +		unsigned int base, u16 *res);
> +int __must_check kstrtos16_from_user(const char __user *s, size_t count,
> +		unsigned int base, s16 *res);
> +int __must_check kstrtou8_from_user(const char __user *s, size_t count,
> +		unsigned int base, u8 *res);
> +int __must_check kstrtos8_from_user(const char __user *s, size_t count,
> +		unsigned int base, s8 *res);
> +int __must_check kstrtobool_from_user(const char __user *s, size_t count,
> +		bool *res);
> +

And this is the perfect example that breaking those lines is a very bad idea.
What you have now is an unreadable mess vs before where you could actually keep
apart function names from arguments.

And, for the record, if we have to break a function signature, we align the
arguments at the opening brace like this:

static inline int __must_check kstrtou64_from_user(const char __user *s,
						   size_t count, unsigned int base, u64 *res)


But if you really want to get your hands dirty with the kernel, I'd
advise to start here: https://kernelnewbies.org/

HTH.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--