Re: [PATCH v5 15/19] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
From: Fabio M. De Francesco <hidden>
Date: 2021-09-15 14:59:11
Also in:
lkml
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:quoted
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.quoted
+ memcpy(data, io_buf, len); + } else { + /* errors */ if (status < 0) { - if (status == (-ESHUTDOWN) ||
status == -ENODEV) {quoted
+ 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