Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1340081 > unrolled thread

[PATCH] Staging: comedi: mite: added spaces around | and *

Back to article view | Back to linux.kernel

Contents

  [PATCH] Staging: comedi: mite: added spaces around | and * Philippe Loctaux <phil@philippeloctaux.com> - 2016-02-23 00:40 +0100
    Re: [PATCH] Staging: comedi: mite: added spaces around | and * Joe Perches <joe@perches.com> - 2016-02-23 01:40 +0100
      Re: [PATCH] Staging: comedi: mite: added spaces around | and * Ian Abbott <abbotti@mev.co.uk> - 2016-02-23 12:10 +0100
        Re: [PATCH] Staging: comedi: mite: added spaces around | and * Philippe Loctaux <phil@philippeloctaux.com> - 2016-02-23 15:00 +0100
          Re: [PATCH] Staging: comedi: mite: added spaces around | and * Ian Abbott <abbotti@mev.co.uk> - 2016-02-23 17:40 +0100
    Re: [PATCH] Staging: comedi: mite: added spaces around | and * Ian Abbott <abbotti@mev.co.uk> - 2016-02-23 17:50 +0100
      Re: [PATCH] Staging: comedi: mite: added spaces around | and * Philippe Loctaux <phil@philippeloctaux.com> - 2016-02-24 00:10 +0100

#1340081 — [PATCH] Staging: comedi: mite: added spaces around | and *

Added spaces around | and *, fixing 2 checkpatch checks.

Signed-off-by: Philippe Loctaux <phil@philippeloctaux.com>
---
 drivers/staging/comedi/drivers/mite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/mite.c b/drivers/staging/comedi/drivers/mite.c
index 8f24702..d5cd4f3 100644
--- a/drivers/staging/comedi/drivers/mite.c
+++ b/drivers/staging/comedi/drivers/mite.c
@@ -51,7 +51,7 @@
 
 #include "mite.h"
 
-#define TOP_OF_PAGE(x) ((x)|(~(PAGE_MASK)))
+#define TOP_OF_PAGE(x) ((x) | (~(PAGE_MASK)))
 
 struct mite_struct *mite_alloc(struct pci_dev *pcidev)
 {
@@ -216,7 +216,7 @@ EXPORT_SYMBOL_GPL(mite_free_ring);
 struct mite_channel *mite_request_channel_in_range(struct mite_struct *mite,
 						   struct
 						   mite_dma_descriptor_ring
-						   *ring, unsigned min_channel,
+						   * ring, unsigned min_channel,
 						   unsigned max_channel)
 {
 	int i;
-- 
2.7.1

[toc] | [next] | [standalone]

#1340102

On Tue, 2016-02-23 at 00:38 +0100, Philippe Loctaux wrote:
> Added spaces around | and *, fixing 2 checkpatch checks.
[]
> diff --git a/drivers/staging/comedi/drivers/mite.c b/drivers/staging/comedi/drivers/mite.c
[]
> @@ -51,7 +51,7 @@
>  
>  #include "mite.h"
>  
> -#define TOP_OF_PAGE(x) ((x)|(~(PAGE_MASK)))
> +#define TOP_OF_PAGE(x) ((x) | (~(PAGE_MASK)))

It'd be nice to remove superfluous parentheses here too

+#define TOP_OF_PAGE(x) ((x) | ~PAGE_MASK)

[toc] | [prev] | [next] | [standalone]

#1340548

On 23/02/16 00:37, Joe Perches wrote:
> On Tue, 2016-02-23 at 00:38 +0100, Philippe Loctaux wrote:
>> Added spaces around | and *, fixing 2 checkpatch checks.
> []
>> diff --git a/drivers/staging/comedi/drivers/mite.c b/drivers/staging/comedi/drivers/mite.c
> []
>> @@ -51,7 +51,7 @@
>>
>>   #include "mite.h"
>>
>> -#define TOP_OF_PAGE(x) ((x)|(~(PAGE_MASK)))
>> +#define TOP_OF_PAGE(x) ((x) | (~(PAGE_MASK)))
>
> It'd be nice to remove superfluous parentheses here too
>
> +#define TOP_OF_PAGE(x) ((x) | ~PAGE_MASK)
>

Even better: just remove the macro, as it isn't used.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

[toc] | [prev] | [next] | [standalone]

#1340634

On Tue, Feb 23, 2016 at 11:07:15AM +0000, Ian Abbott wrote:
> Even better: just remove the macro, as it isn't used.
How can I do that? I'm asking that because my C knowledge
isn't very good :P

--
Philippe Loctaux
phil@philippeloctaux.com

[toc] | [prev] | [next] | [standalone]

#1340849

On 23/02/16 12:38, Philippe Loctaux wrote:
> On Tue, Feb 23, 2016 at 11:07:15AM +0000, Ian Abbott wrote:
>> Even better: just remove the macro, as it isn't used.
> How can I do that? I'm asking that because my C knowledge
> isn't very good :P

Are you sure you should be writing kernel patches in that case?

It means, just delete the macro definition.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

[toc] | [prev] | [next] | [standalone]

#1340855

On 22/02/16 23:38, Philippe Loctaux wrote:
> Added spaces around | and *, fixing 2 checkpatch checks.
>
> Signed-off-by: Philippe Loctaux <phil@philippeloctaux.com>
> ---
>   drivers/staging/comedi/drivers/mite.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
[snip]
> @@ -216,7 +216,7 @@ EXPORT_SYMBOL_GPL(mite_free_ring);
>   struct mite_channel *mite_request_channel_in_range(struct mite_struct *mite,
>   						   struct
>   						   mite_dma_descriptor_ring
> -						   *ring, unsigned min_channel,
> +						   * ring, unsigned min_channel,
>   						   unsigned max_channel)

Actually, that one looks like a false positive by the checkpatch tool. 
I'd expect no space after that '*'.  checkpatch has probably got 
confused by the line-break before the '*'.

It looks a bit ugly anyway.  It would look better if there was a 
line-break before the function name, the same as in 
"drivers/staging/comedi/drivers/mite.h":

struct mite_channel *
mite_request_channel_in_range(struct mite_struct *mite,
			      struct mite_dma_descriptor_ring *ring,
			      unsigned min_channel, unsigned max_channel)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-

[toc] | [prev] | [next] | [standalone]

#1341157

On Tue, Feb 23, 2016 at 04:48:23PM +0000, Ian Abbott wrote:
> Actually, that one looks like a false positive by the checkpatch tool. I'd
> expect no space after that '*'.  checkpatch has probably got confused by the
> line-break before the '*'.
I see, I think I'll end up resending another patch without
changing this line, to avoid any bugs or breaking some style rules.

--
Philippe Loctaux
phil@philippeloctaux.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web