diff mbox

[OPW,kernel,1/4] staging: rtl8187se: use gfp_t instead of int

Message ID 1382206188-12297-2-git-send-email-teobaluta@gmail.com
State New, archived
Headers show

Commit Message

Teodora Baluta Oct. 19, 2013, 6:09 p.m. UTC
Fix the following type of sparse warnings:

drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17: warning: incorrect type in argument 2 (different base types)
drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    expected restricted gfp_t [usertype] flags
drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    got int [signed] gfp_mask

Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
---
 drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Waskiewicz Jr, Peter P Oct. 20, 2013, 12:32 a.m. UTC | #1
On Sat, 2013-10-19 at 21:09 +0300, Teodora Baluta wrote:
> Fix the following type of sparse warnings:
> 
> drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17: warning: incorrect type in argument 2 (different base types)
> drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    expected restricted gfp_t [usertype] flags
> drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    got int [signed] gfp_mask

These commit lines way exceed the 80 character limit.  It may be better
to just include the filename, or something like this:

In drivers/staging/rtl8187se/ieee80211:

ieee80211_tx.c:249:17: warning: incorrect type in argument 2 (different
base types)
ieee80211_tx.c:249:17: expected restricted gfp_t [usertype] flags
ieee80211_tx.c:249:17: got int [signed] gfp_mask

Otherwise the patch looks fine.

-PJ

> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
> ---
>  drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> index b346653..bc338a5 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> @@ -240,7 +240,7 @@ void ieee80211_txb_free(struct ieee80211_txb *txb) {
>  }
>  
>  struct ieee80211_txb *ieee80211_alloc_txb(int nr_frags, int txb_size,
> -					  int gfp_mask)
> +					  gfp_t gfp_mask)
>  {
>  	struct ieee80211_txb *txb;
>  	int i;
> -- 
> 1.7.10.4
>
Teodora Baluta Oct. 20, 2013, 9:17 a.m. UTC | #2
On Sunday, October 20, 2013 3:32:13 AM UTC+3, Waskiewicz Jr, Peter P wrote:

> On Sat, 2013-10-19 at 21:09 +0300, Teodora Baluta wrote: 
> > Fix the following type of sparse warnings: 
> > 
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17: warning: 
> incorrect type in argument 2 (different base types) 
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    expected 
> restricted gfp_t [usertype] flags 
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    got int 
> [signed] gfp_mask 
>
> These commit lines way exceed the 80 character limit.  It may be better 
> to just include the filename, or something like this: 
>
> In drivers/staging/rtl8187se/ieee80211: 
>
> ieee80211_tx.c:249:17: warning: incorrect type in argument 2 (different 
> base types) 
> ieee80211_tx.c:249:17: expected restricted gfp_t [usertype] flags 
> ieee80211_tx.c:249:17: got int [signed] gfp_mask 
>
> Otherwise the patch looks fine. 
>
>
I agree that including just the file name and the warning as you proposed 
is better, but I am a bit confused about this feedback. In another patch I 
send I got feedback not to wrap lines that are compiler messages (
https://groups.google.com/forum/#!topic/opw-kernel/MiEYvp_qzaQ). What is 
the preferred commit message when including compiler warnings? Is it ok to 
include compiler messages at all? 


> > Signed-off-by: Teodora Baluta <teob...@gmail.com <javascript:>> 
> > --- 
> >  drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |    2 +- 
> >  1 file changed, 1 insertion(+), 1 deletion(-) 
> > 
> > diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > index b346653..bc338a5 100644 
> > --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > @@ -240,7 +240,7 @@ void ieee80211_txb_free(struct ieee80211_txb *txb) { 
> >  } 
> >   
> >  struct ieee80211_txb *ieee80211_alloc_txb(int nr_frags, int txb_size, 
> > -                                          int gfp_mask) 
> > +                                          gfp_t gfp_mask) 
> >  { 
> >          struct ieee80211_txb *txb; 
> >          int i; 
> > -- 
> > 1.7.10.4 
> > 
>

This patchset was already added to the staging tree.

Thanks,
Teodora
Waskiewicz Jr, Peter P Oct. 20, 2013, 9:21 a.m. UTC | #3
On Sun, 2013-10-20 at 02:17 -0700, Teodora B?lu?? wrote:
> On Sunday, October 20, 2013 3:32:13 AM UTC+3, Waskiewicz Jr, Peter P
> wrote:
>         On Sat, 2013-10-19 at 21:09 +0300, Teodora Baluta wrote: 
>         > Fix the following type of sparse warnings: 
>         > 
>         > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:
>         warning: incorrect type in argument 2 (different base types) 
>         > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:
>            expected restricted gfp_t [usertype] flags 
>         > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:
>            got int [signed] gfp_mask 
>         
>         These commit lines way exceed the 80 character limit.  It may
>         be better 
>         to just include the filename, or something like this: 
>         
>         In drivers/staging/rtl8187se/ieee80211: 
>         
>         ieee80211_tx.c:249:17: warning: incorrect type in argument 2
>         (different 
>         base types) 
>         ieee80211_tx.c:249:17: expected restricted gfp_t [usertype]
>         flags 
>         ieee80211_tx.c:249:17: got int [signed] gfp_mask 
>         
>         Otherwise the patch looks fine. 
>         
> 
> 
> I agree that including just the file name and the warning as you
> proposed is better, but I am a bit confused about this feedback. In
> another patch I send I got feedback not to wrap lines that are
> compiler messages (https://groups.google.com/forum/#!
> topic/opw-kernel/MiEYvp_qzaQ). What is the preferred commit message
> when including compiler warnings? Is it ok to include compiler
> messages at all? 

And this is why we have multiple people review things, because after
reading Josh's reply to you, I agree with him.  :-)

> 
>         
>         > Signed-off-by: Teodora Baluta <teob...@gmail.com> 
>         > --- 
>         >  drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |    2
>         +- 
>         >  1 file changed, 1 insertion(+), 1 deletion(-) 
>         > 
>         > diff --git
>         a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
>         b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
>         > index b346653..bc338a5 100644 
>         > --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
>         > +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
>         > @@ -240,7 +240,7 @@ void ieee80211_txb_free(struct
>         ieee80211_txb *txb) { 
>         >  } 
>         >   
>         >  struct ieee80211_txb *ieee80211_alloc_txb(int nr_frags, int
>         txb_size, 
>         > -                                          int gfp_mask) 
>         > +                                          gfp_t gfp_mask) 
>         >  { 
>         >          struct ieee80211_txb *txb; 
>         >          int i; 
>         > -- 
>         > 1.7.10.4 
>         > 
> 
> 
> This patchset was already added to the staging tree.
> 
> 
> Thanks,
> Teodora 
>
Josh Triplett Oct. 20, 2013, 12:54 p.m. UTC | #4
On Sun, Oct 20, 2013 at 12:32:13AM +0000, Waskiewicz Jr, Peter P wrote:
> On Sat, 2013-10-19 at 21:09 +0300, Teodora Baluta wrote:
> > Fix the following type of sparse warnings:
> > 
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17: warning: incorrect type in argument 2 (different base types)
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    expected restricted gfp_t [usertype] flags
> > drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c:249:17:    got int [signed] gfp_mask
> 
> These commit lines way exceed the 80 character limit.
[...snip...]

Pasting in compiler output and other programmatic output is one of the
standard exceptions to the line-length rules for commit messages.  I'd
really like to continue seeing the full untruncated warnings, please.

This patch looks good to me.

Reviewed-by: Josh Triplett <josh@joshtriplett.org>
diff mbox

Patch

diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
index b346653..bc338a5 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
@@ -240,7 +240,7 @@  void ieee80211_txb_free(struct ieee80211_txb *txb) {
 }
 
 struct ieee80211_txb *ieee80211_alloc_txb(int nr_frags, int txb_size,
-					  int gfp_mask)
+					  gfp_t gfp_mask)
 {
 	struct ieee80211_txb *txb;
 	int i;