On Wednesday, September 15, 2021 3:53:01 PM CEST Dan Carpenter wrote:
> On Wed, Sep 15, 2021 at 02:41:45PM +0200, Fabio M. De Francesco wrote:
> > Clean up usbctrl_vendoreq () in usb_ops_linux.c. Eventually this function
> > will be deleted but some of the code will be reused later. This cleanup
> > makes code reuse easier.
> >
> Thanks for removing the URL. This commit message is no longer bad to
> the point where it has to be redone but it's still not great.
>
> I explicitly told you to leave the irrelevant information out. I'm
> trying to help you and it's frustrating that you're not listening. I
> wish that you had just copy and pasted the commit message which I sent.
I'm sorry, seriously. It's hard to listen carefully when I need to do my
*real* work while trying my best to contribute to the kernel. Sometimes I'm
so tired that I forget something important or what it is said by reviewers. I
know that this is not a good excuse, anyway please don't ever think that I
don't mind of the time you spend on reviews and writing suggestions.
> This relates the discussion we had about reviewing patches one at a time
> in the order they arrive. Every patch should be self contained. It
> should not refer to the past except in the case of explaining the Fixes
> tag and it should not refer to the future except in the case where it
> needs to excuse adding unused infrastructure. Reviewing is stateless.
> We don't want to know about your plans.
>
> On the other hand, the commit message doesn't list the changes the
> commit makes as part of the clean up process. That would have been
> helpful information for me as a reviewer.
>
> *Sigh* Whatever... I would have allowed this commit message but there
> is a bug in the code.
>
> > + memcpy(data, io_buf, len);
> > + } else {
> > + /* errors */
> > if (status < 0) {
> > - if (status == (-ESHUTDOWN) ||
status == -ENODEV) {
> > + if (status == (-ESHUTDOWN || -
ENODEV)) {
>
> This is a bug so you'll have to redo the patch.
This is the proof of what I was trying to convey with the words above. I
perfectly knew, since days, that this line is wrong but for some reason that
I really cannot understand why it's still there.
Thank you very much,
Fabio
> regards,
> dan carpenter
>
