linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] linux/kernel.h: break long lines
@ 2018-02-24 18:10 Gustavo Leite
  2018-02-24 18:30 ` Borislav Petkov
  0 siblings, 1 reply; 5+ messages in thread
From: Gustavo Leite @ 2018-02-24 18:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Gustavo Leite, Andrew Morton, Ingo Molnar, Thomas Gleixner,
	Kees Cook, Baoquan He, Ian Abbott, Borislav Petkov, Randy Dunlap,
	Niklas Söderlund

Break lines that are longer than 80 characters do match the linux coding
style.

Signed-off-by: Gustavo Leite <gustavoleite.ti@gmail.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);
 
-int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
+int __must_check kstrtoull(const char *s, unsigned int base,
+		unsigned long long *res);
 int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
 
 /**
@@ -329,7 +331,8 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
  * Used as a replacement for the obsolete simple_strtoull. Return code must
  * be checked.
 */
-static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
+static inline int __must_check kstrtoul(const char *s, unsigned int base,
+		unsigned long *res)
 {
 	/*
 	 * We want to shortcut function call, but
@@ -358,7 +361,8 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
  * Used as a replacement for the obsolete simple_strtoull. Return code must
  * be checked.
  */
-static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
+static inline int __must_check kstrtol(const char *s, unsigned int base,
+		long *res)
 {
 	/*
 	 * We want to shortcut function call, but
@@ -371,25 +375,30 @@ static inline int __must_check kstrtol(const char *s, unsigned int base, long *r
 		return _kstrtol(s, base, res);
 }
 
-int __must_check kstrtouint(const char *s, unsigned int base, unsigned int *res);
+int __must_check kstrtouint(const char *s, unsigned int base,
+		unsigned int *res);
 int __must_check kstrtoint(const char *s, unsigned int base, int *res);
 
-static inline int __must_check kstrtou64(const char *s, unsigned int base, u64 *res)
+static inline int __must_check kstrtou64(const char *s, unsigned int base,
+		u64 *res)
 {
 	return kstrtoull(s, base, res);
 }
 
-static inline int __must_check kstrtos64(const char *s, unsigned int base, s64 *res)
+static inline int __must_check kstrtos64(const char *s, unsigned int base,
+		s64 *res)
 {
 	return kstrtoll(s, base, res);
 }
 
-static inline int __must_check kstrtou32(const char *s, unsigned int base, u32 *res)
+static inline int __must_check kstrtou32(const char *s, unsigned int base,
+		u32 *res)
 {
 	return kstrtouint(s, base, res);
 }
 
-static inline int __must_check kstrtos32(const char *s, unsigned int base, s32 *res)
+static inline int __must_check kstrtos32(const char *s, unsigned int base,
+		s32 *res)
 {
 	return kstrtoint(s, base, res);
 }
@@ -400,34 +409,49 @@ int __must_check kstrtou8(const char *s, unsigned int base, u8 *res);
 int __must_check kstrtos8(const char *s, unsigned int base, s8 *res);
 int __must_check kstrtobool(const char *s, bool *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);
+
+static inline int __must_check kstrtou64_from_user(const char __user *s,
+		size_t count, unsigned int base, u64 *res)
 {
 	return kstrtoull_from_user(s, count, base, res);
 }
 
-static inline int __must_check kstrtos64_from_user(const char __user *s, size_t count, unsigned int base, s64 *res)
+static inline int __must_check kstrtos64_from_user(const char __user *s,
+		size_t count, unsigned int base, s64 *res)
 {
 	return kstrtoll_from_user(s, count, base, res);
 }
 
-static inline int __must_check kstrtou32_from_user(const char __user *s, size_t count, unsigned int base, u32 *res)
+static inline int __must_check kstrtou32_from_user(const char __user *s,
+		size_t count, unsigned int base, u32 *res)
 {
 	return kstrtouint_from_user(s, count, base, res);
 }
 
-static inline int __must_check kstrtos32_from_user(const char __user *s, size_t count, unsigned int base, s32 *res)
+static inline int __must_check kstrtos32_from_user(const char __user *s,
+		size_t count, unsigned int base, s32 *res)
 {
 	return kstrtoint_from_user(s, count, base, res);
 }
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] linux/kernel.h: break long lines
  2018-02-24 18:10 [PATCH] linux/kernel.h: break long lines Gustavo Leite
@ 2018-02-24 18:30 ` Borislav Petkov
  2018-02-24 18:45   ` Theodore Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2018-02-24 18:30 UTC (permalink / raw)
  To: Gustavo Leite
  Cc: linux-kernel, Andrew Morton, Ingo Molnar, Thomas Gleixner,
	Kees Cook, Baoquan He, Ian Abbott, Randy Dunlap,
	Niklas Söderlund

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@gmail.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)
-- 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] linux/kernel.h: break long lines
  2018-02-24 18:30 ` Borislav Petkov
@ 2018-02-24 18:45   ` Theodore Ts'o
  2018-02-24 18:51     ` Borislav Petkov
  0 siblings, 1 reply; 5+ messages in thread
From: Theodore Ts'o @ 2018-02-24 18:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Gustavo Leite, linux-kernel, Andrew Morton, Ingo Molnar,
	Thomas Gleixner, Kees Cook, Baoquan He, Ian Abbott, Randy Dunlap,
	Niklas Söderlund

On Sat, Feb 24, 2018 at 07:30:44PM +0100, Borislav Petkov wrote:
> 
> 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)
>

An alternate approach is this:

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

Which yes, is still longer than 80 characters.  But this is where
blindly following coding guidelines as if they are fundamentalist
biblical doctrine is not really a great idea.  The goal is to make the
code easier to read, and very often it's important to apply _judgement_.
(Which is one of the reasons why I generally don't really like newbies
trying to apply checkpatch.pl to existing source files.)

Cheers,

					- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] linux/kernel.h: break long lines
  2018-02-24 18:45   ` Theodore Ts'o
@ 2018-02-24 18:51     ` Borislav Petkov
  0 siblings, 0 replies; 5+ messages in thread
From: Borislav Petkov @ 2018-02-24 18:51 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Gustavo Leite, linux-kernel, Andrew Morton, Ingo Molnar,
	Thomas Gleixner, Kees Cook, Baoquan He, Ian Abbott, Randy Dunlap,
	Niklas Söderlund

On Sat, Feb 24, 2018 at 01:45:30PM -0500, Theodore Ts'o wrote:
> static inline int __must_check
> kstrtou64_from_user(const char __user *s, size_t count, unsigned int base, u64 *res)

Yeah, had already typed that one in the reply but then opted for not
mentioning it because it is a bit controversial with maintainers. :)

> Which yes, is still longer than 80 characters.  But this is where
> blindly following coding guidelines as if they are fundamentalist
> biblical doctrine is not really a great idea.  The goal is to make the
> code easier to read, and very often it's important to apply _judgement_.
> (Which is one of the reasons why I generally don't really like newbies
> trying to apply checkpatch.pl to existing source files.)

That's basically what I was trying to say but your formulation is much
better.

Thx.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] linux/kernel.h: break long lines
@ 2018-02-24 21:17 Alexey Dobriyan
  0 siblings, 0 replies; 5+ messages in thread
From: Alexey Dobriyan @ 2018-02-24 21:17 UTC (permalink / raw)
  To: gustavoleite.ti; +Cc: linux-kernel

> -static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
> +static inline int __must_check kstrtol(const char *s, unsigned int base,
> +		long *res)

Please find and fix a bug instead of making feel-good changes.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-02-24 21:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-24 18:10 [PATCH] linux/kernel.h: break long lines Gustavo Leite
2018-02-24 18:30 ` Borislav Petkov
2018-02-24 18:45   ` Theodore Ts'o
2018-02-24 18:51     ` Borislav Petkov
2018-02-24 21:17 Alexey Dobriyan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).