* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:45 [RFC] Cleanup line-wrapping in pgtable.h Adam Litke
@ 2005-08-17 17:56 ` Christoph Hellwig
2005-08-17 19:48 ` Alexey Dobriyan
2005-08-17 18:00 ` Tommy Reynolds
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2005-08-17 17:56 UTC (permalink / raw)
To: Adam Litke; +Cc: linux-kernel
> +static inline int pte_user(pte_t pte)
> + { return (pte).pte_low & _PAGE_USER; }
Once you start reformatting things please make sure the result version
matches the documented codingstyle. That would be:
static inline int pte_user(pte_t pte)
{
return (pte).pte_low & _PAGE_USER;
}
Quite a bit more verbose, but also a lot better readable.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:56 ` Christoph Hellwig
@ 2005-08-17 19:48 ` Alexey Dobriyan
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Dobriyan @ 2005-08-17 19:48 UTC (permalink / raw)
To: Adam Litke; +Cc: Christoph Hellwig, linux-kernel
On Wed, Aug 17, 2005 at 06:56:01PM +0100, Christoph Hellwig wrote:
> > +static inline int pte_user(pte_t pte)
> > + { return (pte).pte_low & _PAGE_USER; }
>
> Once you start reformatting things please make sure the result version
> matches the documented codingstyle. That would be:
>
> static inline int pte_user(pte_t pte)
> {
> return (pte).pte_low & _PAGE_USER;
^ ^
> }
>
> Quite a bit more verbose, but also a lot better readable.
Don't forget to drop "(" and ")".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:45 [RFC] Cleanup line-wrapping in pgtable.h Adam Litke
2005-08-17 17:56 ` Christoph Hellwig
@ 2005-08-17 18:00 ` Tommy Reynolds
2005-08-17 18:19 ` Dave Hansen
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Tommy Reynolds @ 2005-08-17 18:00 UTC (permalink / raw)
To: Adam Litke; +Cc: linux-kernel, agl
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
Uttered Adam Litke <agl@us.ibm.com>, spake thus:
> The line-wrapping in most of the include/asm/pgtable.h pte test/set
> macros looks horrible in my 80 column terminal. The following "test the
> waters" patch is how I would like to see them laid out. I realize that
> the braces don't adhere to CodingStyle but the advantage is (when taking
> wrapping into account) that the code takes up no additional space. How
> do people feel about making this change? Any better suggestions? I
> personally wouldn't like a lone closing brace like normal functions
> because of the extra lines eaten. I volunteer to patch up the other
> architectures if we reach a consensus.
Congratulations for keeping your 80-column display. The coding
standard assumes that and 8-character tabs for carefully thought-out
software development reasons -- they are functional, not traditional.
Since these are inline procedures, make them follow the coding
standard; that's why we have one.
Cheers
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:45 [RFC] Cleanup line-wrapping in pgtable.h Adam Litke
2005-08-17 17:56 ` Christoph Hellwig
2005-08-17 18:00 ` Tommy Reynolds
@ 2005-08-17 18:19 ` Dave Hansen
2005-08-22 9:29 ` Pavel Machek
2005-08-22 21:11 ` Matthew Helsley
4 siblings, 0 replies; 7+ messages in thread
From: Dave Hansen @ 2005-08-17 18:19 UTC (permalink / raw)
To: Adam Litke; +Cc: Linux Kernel Mailing List
On Wed, 2005-08-17 at 12:45 -0500, Adam Litke wrote:
> The line-wrapping in most of the include/asm/pgtable.h pte test/set
> macros looks horrible in my 80 column terminal. The following "test the
> waters" patch is how I would like to see them laid out. I realize that
> the braces don't adhere to CodingStyle but the advantage is (when taking
> wrapping into account) that the code takes up no additional space. How
> do people feel about making this change? Any better suggestions? I
> personally wouldn't like a lone closing brace like normal functions
> because of the extra lines eaten. I volunteer to patch up the other
> architectures if we reach a consensus.
I'd probably just leave it alone. Those are things that are virtually
never touched, and are quite unlikely to have any bugs in them.
But, if you do decide to change them, it might also be a nice idea to
consolidate some of the ones that are duplicated across architectures.
They could probably even go into the existing
include/asm-generic/pgtable.h. Please use full, proper CodingStyle if
you do decide to split them across several lines.
-- Dave
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:45 [RFC] Cleanup line-wrapping in pgtable.h Adam Litke
` (2 preceding siblings ...)
2005-08-17 18:19 ` Dave Hansen
@ 2005-08-22 9:29 ` Pavel Machek
2005-08-22 21:11 ` Matthew Helsley
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Machek @ 2005-08-22 9:29 UTC (permalink / raw)
To: Adam Litke; +Cc: linux-kernel
Hi!
> The line-wrapping in most of the include/asm/pgtable.h pte test/set
> macros looks horrible in my 80 column terminal. The following "test the
> waters" patch is how I would like to see them laid out. I realize that
> the braces don't adhere to CodingStyle but the advantage is (when taking
> wrapping into account) that the code takes up no additional space. How
> do people feel about making this change? Any better suggestions? I
> personally wouldn't like a lone closing brace like normal functions
> because of the extra lines eaten. I volunteer to patch up the other
> architectures if we reach a consensus.
I'm afraid that original version looks better than new one...
Pavel
> Signed-off-by: Adam Litke <agl@us.ibm.com>
>
> pgtable.h | 51 ++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 34 insertions(+), 17 deletions(-)
> diff -upN reference/include/asm-i386/pgtable.h current/include/asm-i386/pgtable.h
> --- reference/include/asm-i386/pgtable.h
> +++ current/include/asm-i386/pgtable.h
> @@ -215,28 +215,45 @@ extern unsigned long pg0[];
> * The following only work if pte_present() is true.
> * Undefined behaviour if not..
> */
> -static inline int pte_user(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
> -static inline int pte_read(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
> -static inline int pte_dirty(pte_t pte) { return (pte).pte_low & _PAGE_DIRTY; }
> -static inline int pte_young(pte_t pte) { return (pte).pte_low & _PAGE_ACCESSED; }
> -static inline int pte_write(pte_t pte) { return (pte).pte_low & _PAGE_RW; }
> +static inline int pte_user(pte_t pte)
> + { return (pte).pte_low & _PAGE_USER; }
> +static inline int pte_read(pte_t pte)
> + { return (pte).pte_low & _PAGE_USER; }
> +static inline int pte_dirty(pte_t pte)
> + { return (pte).pte_low & _PAGE_DIRTY; }
> +static inline int pte_young(pte_t pte)
> + { return (pte).pte_low & _PAGE_ACCESSED; }
> +static inline int pte_write(pte_t pte)
> + { return (pte).pte_low & _PAGE_RW; }
>
> /*
> * The following only works if pte_present() is not true.
> */
> -static inline int pte_file(pte_t pte) { return (pte).pte_low & _PAGE_FILE; }
> +static inline int pte_file(pte_t pte)
> + { return (pte).pte_low & _PAGE_FILE; }
>
> -static inline pte_t pte_rdprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
> -static inline pte_t pte_exprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
> -static inline pte_t pte_mkclean(pte_t pte) { (pte).pte_low &= ~_PAGE_DIRTY; return pte; }
> -static inline pte_t pte_mkold(pte_t pte) { (pte).pte_low &= ~_PAGE_ACCESSED; return pte; }
> -static inline pte_t pte_wrprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_RW; return pte; }
> -static inline pte_t pte_mkread(pte_t pte) { (pte).pte_low |= _PAGE_USER; return pte; }
> -static inline pte_t pte_mkexec(pte_t pte) { (pte).pte_low |= _PAGE_USER; return pte; }
> -static inline pte_t pte_mkdirty(pte_t pte) { (pte).pte_low |= _PAGE_DIRTY; return pte; }
> -static inline pte_t pte_mkyoung(pte_t pte) { (pte).pte_low |= _PAGE_ACCESSED; return pte; }
> -static inline pte_t pte_mkwrite(pte_t pte) { (pte).pte_low |= _PAGE_RW; return pte; }
> -static inline pte_t pte_mkhuge(pte_t pte) { (pte).pte_low |= _PAGE_PRESENT | _PAGE_PSE; return pte; }
> +static inline pte_t pte_rdprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_USER; return pte; }
> +static inline pte_t pte_exprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_USER; return pte; }
> +static inline pte_t pte_mkclean(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_DIRTY; return pte; }
> +static inline pte_t pte_mkold(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_ACCESSED; return pte; }
> +static inline pte_t pte_wrprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_RW; return pte; }
> +static inline pte_t pte_mkread(pte_t pte)
> + { (pte).pte_low |= _PAGE_USER; return pte; }
> +static inline pte_t pte_mkexec(pte_t pte)
> + { (pte).pte_low |= _PAGE_USER; return pte; }
> +static inline pte_t pte_mkdirty(pte_t pte)
> + { (pte).pte_low |= _PAGE_DIRTY; return pte; }
> +static inline pte_t pte_mkyoung(pte_t pte)
> + { (pte).pte_low |= _PAGE_ACCESSED; return pte; }
> +static inline pte_t pte_mkwrite(pte_t pte)
> + { (pte).pte_low |= _PAGE_RW; return pte; }
> +static inline pte_t pte_mkhuge(pte_t pte)
> + { (pte).pte_low |= _PAGE_PRESENT | _PAGE_PSE; return pte; }
>
> #ifdef CONFIG_X86_PAE
> # include <asm/pgtable-3level.h>
--
if you have sharp zaurus hardware you don't need... you know my address
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC] Cleanup line-wrapping in pgtable.h
2005-08-17 17:45 [RFC] Cleanup line-wrapping in pgtable.h Adam Litke
` (3 preceding siblings ...)
2005-08-22 9:29 ` Pavel Machek
@ 2005-08-22 21:11 ` Matthew Helsley
4 siblings, 0 replies; 7+ messages in thread
From: Matthew Helsley @ 2005-08-22 21:11 UTC (permalink / raw)
To: Adam Litke; +Cc: LKML
On Wed, 2005-08-17 at 12:45 -0500, Adam Litke wrote:
> The line-wrapping in most of the include/asm/pgtable.h pte test/set
> macros looks horrible in my 80 column terminal. The following "test the
> waters" patch is how I would like to see them laid out. I realize that
> the braces don't adhere to CodingStyle but the advantage is (when taking
> wrapping into account) that the code takes up no additional space. How
> do people feel about making this change? Any better suggestions? I
> personally wouldn't like a lone closing brace like normal functions
> because of the extra lines eaten. I volunteer to patch up the other
> architectures if we reach a consensus.
>
> Signed-off-by: Adam Litke <agl@us.ibm.com>
>
> pgtable.h | 51 ++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 34 insertions(+), 17 deletions(-)
> diff -upN reference/include/asm-i386/pgtable.h current/include/asm-i386/pgtable.h
> --- reference/include/asm-i386/pgtable.h
> +++ current/include/asm-i386/pgtable.h
> @@ -215,28 +215,45 @@ extern unsigned long pg0[];
> * The following only work if pte_present() is true.
> * Undefined behaviour if not..
> */
> -static inline int pte_user(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
> -static inline int pte_read(pte_t pte) { return (pte).pte_low & _PAGE_USER; }
> -static inline int pte_dirty(pte_t pte) { return (pte).pte_low & _PAGE_DIRTY; }
> -static inline int pte_young(pte_t pte) { return (pte).pte_low & _PAGE_ACCESSED; }
> -static inline int pte_write(pte_t pte) { return (pte).pte_low & _PAGE_RW; }
> +static inline int pte_user(pte_t pte)
> + { return (pte).pte_low & _PAGE_USER; }
> +static inline int pte_read(pte_t pte)
> + { return (pte).pte_low & _PAGE_USER; }
> +static inline int pte_dirty(pte_t pte)
> + { return (pte).pte_low & _PAGE_DIRTY; }
> +static inline int pte_young(pte_t pte)
> + { return (pte).pte_low & _PAGE_ACCESSED; }
> +static inline int pte_write(pte_t pte)
> + { return (pte).pte_low & _PAGE_RW; }
I think removing the whitespace preceding the opening braces is closer
to CodingStyle, allows for longer lines in the future (however gross
they may be), and does not alter the vertical space consumed on your
display.
> /*
> * The following only works if pte_present() is not true.
> */
> -static inline int pte_file(pte_t pte) { return (pte).pte_low & _PAGE_FILE; }
> +static inline int pte_file(pte_t pte)
> + { return (pte).pte_low & _PAGE_FILE; }
>
> -static inline pte_t pte_rdprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
> -static inline pte_t pte_exprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_USER; return pte; }
> -static inline pte_t pte_mkclean(pte_t pte) { (pte).pte_low &= ~_PAGE_DIRTY; return pte; }
> -static inline pte_t pte_mkold(pte_t pte) { (pte).pte_low &= ~_PAGE_ACCESSED; return pte; }
> -static inline pte_t pte_wrprotect(pte_t pte) { (pte).pte_low &= ~_PAGE_RW; return pte; }
As long as this file is being polished, you might move pte_wrprotect()
up so it is between pte_rdprotect() and pte_exprotect().
> -static inline pte_t pte_mkread(pte_t pte) { (pte).pte_low |= _PAGE_USER; return pte; }
> -static inline pte_t pte_mkexec(pte_t pte) { (pte).pte_low |= _PAGE_USER; return pte; }
> -static inline pte_t pte_mkdirty(pte_t pte) { (pte).pte_low |= _PAGE_DIRTY; return pte; }
> -static inline pte_t pte_mkyoung(pte_t pte) { (pte).pte_low |= _PAGE_ACCESSED; return pte; }
> -static inline pte_t pte_mkwrite(pte_t pte) { (pte).pte_low |= _PAGE_RW; return pte; }
> -static inline pte_t pte_mkhuge(pte_t pte) { (pte).pte_low |= _PAGE_PRESENT | _PAGE_PSE; return pte; }
> +static inline pte_t pte_rdprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_USER; return pte; }
> +static inline pte_t pte_exprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_USER; return pte; }
> +static inline pte_t pte_mkclean(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_DIRTY; return pte; }
> +static inline pte_t pte_mkold(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_ACCESSED; return pte; }
> +static inline pte_t pte_wrprotect(pte_t pte)
> + { (pte).pte_low &= ~_PAGE_RW; return pte; }
> +static inline pte_t pte_mkread(pte_t pte)
> + { (pte).pte_low |= _PAGE_USER; return pte; }
> +static inline pte_t pte_mkexec(pte_t pte)
> + { (pte).pte_low |= _PAGE_USER; return pte; }
> +static inline pte_t pte_mkdirty(pte_t pte)
> + { (pte).pte_low |= _PAGE_DIRTY; return pte; }
> +static inline pte_t pte_mkyoung(pte_t pte)
> + { (pte).pte_low |= _PAGE_ACCESSED; return pte; }
> +static inline pte_t pte_mkwrite(pte_t pte)
> + { (pte).pte_low |= _PAGE_RW; return pte; }
> +static inline pte_t pte_mkhuge(pte_t pte)
> + { (pte).pte_low |= _PAGE_PRESENT | _PAGE_PSE; return pte; }
>
> #ifdef CONFIG_X86_PAE
> # include <asm/pgtable-3level.h>
Cheers,
-Matt Helsley
^ permalink raw reply [flat|nested] 7+ messages in thread