RE: RE: [PATCH v2] usb: core:hub.c: do not print error log on -enomem

RE: RE: [PATCH v2] usb: core:hub.c: do not print error log on -enomem

[Atul Raj]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

 changes in v2
        - added braces as suggested.

 drivers/usb/core/hub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 749d595..82059f26 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -816,9 +816,10 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
         * there can be many TTs per hub).  even if they're uncommon.
         */
        clear = kmalloc(sizeof *clear, GFP_ATOMIC);
-       if (clear == NULL)
+       if (clear == NULL) {
                /* FIXME recover somehow ... RESET_TT? */
                return -ENOMEM;
+       }

        /* info that CLEAR_TT_BUFFER needs */
        clear->tt = tt->multi ? udev->ttport : 1;
--
2.10.2.windows.1



> s/enomem/ENOMEM
 >  
 > On 30-11-16, 06:32, Atul Raj wrote:
 > > All kmalloc-based functions print enough information on failures.
 > > 
 > > Signed-off-by: Atul Raj <atul.r@samsung.com>
 > > ---
 > >  drivers/usb/core/hub.c | 4 +---
 > >  1 file changed, 1 insertion(+), 3 deletions(-)
 > > 
 > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 > > index cbb1467..749d595 100644
 > > --- a/drivers/usb/core/hub.c
 > > +++ b/drivers/usb/core/hub.c
 > > @@ -816,11 +816,9 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
 > >          * there can be many TTs per hub).  even if they're uncommon.
 > >          */
 > >         clear = kmalloc(sizeof *clear, GFP_ATOMIC);
 > > -       if (clear == NULL) {
 > > -               dev_err(&udev->dev, "can't save CLEAR_TT_BUFFER state\n");
 > > +       if (clear == NULL)
 > >                 /* FIXME recover somehow ... RESET_TT? */
 > >                 return -ENOMEM;
 >  
 > It is still preferred to keep {} here as comment also takes a line.
 >  
 > > -       }
 > > 
 > >         /* info that CLEAR_TT_BUFFER needs */
 > >         clear->tt = tt->multi ? udev->ttport : 1;
 > > --
 > > 2.10.2.windows.1
 >  
 >  
 > -- 
 > viresh
 >  
 >

[PATCH] usb:hub: readibility and consistency in coding style

[Atul Raj]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

It is suggested to keep braces if there is a comment in if case as
comment also takes a line.
---
 drivers/usb/core/hub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 82059f26..f43e6302 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5334,10 +5334,10 @@ static int descriptors_changed(struct usb_device *udev,
        }

        buf = kmalloc(len, GFP_NOIO);
-       if (!buf)
+       if (!buf) {
                /* assume the worst */
                return 1;
-
+       }
        for (index = 0; index < udev->descriptor.bNumConfigurations; index++) {
                old_length = le16_to_cpu(udev->config[index].desc.wTotalLength);
                length = usb_get_descriptor(udev, USB_DT_CONFIG, index, buf,
--
2.10.2.windows.1

Re: RE: [PATCH v2] usb: core:hub.c: do not print error log on -enomem

[gregkh]

On Wed, Nov 30, 2016 at 11:40:28AM +0000, Atul Raj wrote:
>  changes in v2
>         - added braces as suggested.

Why is this here?

> 
>  drivers/usb/core/hub.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

No signed off?  You didn't cc: linux-usb@vger?

Come on, please step back and redo this properly, this isn't fun...

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] usb:hub: readibility and consistency in coding style

[gregkh]

On Wed, Nov 30, 2016 at 11:51:39AM +0000, Atul Raj wrote:
> It is suggested to keep braces if there is a comment in if case as
> comment also takes a line.
> ---
>  drivers/usb/core/hub.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Ok, this is getting to be a joke.

Take a few days off, relax, then try to send a patch that works to
yourself first, and run it through scripts/checkpatch.pl to verify that
it really is correct.  Hint, this one isn't at all.

Then, maybe you should start doing some work on drivers/staging/ first,
which is where new developers should start out to work out all of these
issues so you don't bother the other developers who are working on
adding new features and fixing bugs.

Also, why are you including a ton of people who have nothing to do with
usb patches on this one?

good luck,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: Re: [PATCH] usb:hub: readibility and consistency in coding style

[Atul Raj]

[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]

Hi Greg,

I am so happy to get a mail from you and I am really sorry Greg that I disappointed you.
I will take utmost care in future.
I will send you correct patch in sometime.
for your ques
"Also, why are you including a ton of people who have nothing to do with
usb patches on this one?"

I got the name of people from get_maintainer.pl
from now I will include only you.
is it ok??


one more thing in another mail you replied to me as
On Wed, Nov 30, 2016 at 11:40:28AM +0000, Atul Raj wrote:
>  changes in v2
>         - added braces as suggested.

Why is this here?

> 
>  drivers/usb/core/hub.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

do you disagree with viresh comment of braces???

Thanks for your words.


Your Fan
Atul Raj


>
 > On Wed, Nov 30, 2016 at 11:51:39AM +0000, Atul Raj wrote:
 > > It is suggested to keep braces if there is a comment in if case as
 > > comment also takes a line.
 > > ---
 > >  drivers/usb/core/hub.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
 >
 > Ok, this is getting to be a joke.
 >
 > Take a few days off, relax, then try to send a patch that works to
 > yourself first, and run it through scripts/checkpatch.pl to verify that
 > it really is correct.  Hint, this one isn't at all.
 >
 > Then, maybe you should start doing some work on drivers/staging/ first,
 > which is where new developers should start out to work out all of these
 > issues so you don't bother the other developers who are working on
 > adding new features and fixing bugs.
 >
 > Also, why are you including a ton of people who have nothing to do with
 > usb patches on this one?
 >
 > good luck,
 >
 > greg k-h
 

 
 

Re: Re: [PATCH] usb:hub: readibility and consistency in coding style

[gregkh]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, Nov 30, 2016 at 12:35:21PM +0000, Atul Raj wrote:
> Hi Greg,
> 
> I am so happy to get a mail from you and I am really sorry Greg that I disappointed you.
> I will take utmost care in future.
> I will send you correct patch in sometime.
> for your ques
> "Also, why are you including a ton of people who have nothing to do with
> usb patches on this one?"
> 
> I got the name of people from get_maintainer.pl

When using get_maintainer.pl on this patch it does not include the
dri-devel list, so I don't think you used it correctly.

> from now I will include only you.
> is it ok??

No, do it correctly.

> one more thing in another mail you replied to me as
> On Wed, Nov 30, 2016 at 11:40:28AM +0000, Atul Raj wrote:
> >  changes in v2
> >         - added braces as suggested.
> 
> Why is this here?
> 
> > 
> >  drivers/usb/core/hub.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> do you disagree with viresh comment of braces???

I disagree that this is a valid changelog comment, please read
Documentation/SubmittingPatches for the correct way to do this.

Again, if you don't have experience doing this, please work in the
drivers/staging/ directory first, not in the "core" kernel.

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel