Re: Question about rtl8723bs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 22, 2021 at 08:40:11PM -0600, Edmundo Carmona Antoranz wrote:
> Hi!
> 
> I was just trying to correct some compiler warnings coming up when
> building this driver. The one problem that took me into kind of a
> rabbit whole was about having K (in rtw_security.h) set as static. So,
> every time it is included somewhere _and_ K is not being used, the
> compiler is nagging about it. Ok.... easy fix, let's remove the static
> keyword and we should be fine. Then I took a look at _where_ it was
> being used.... well, nowhere.... so, ok, let's get rid of it.... oh,
> here are these macros... are they in use? Actually _they are not_....
> let's remove them also.... (I don't know if you are noticing a pattern
> here) then I noticed that _a lot_ of code has been removed recently
> and so I pulled the big guns and cleaned up the file and started to
> add things back as they were being used in the build process.... in
> the end, around 80 lines are gone (give or take) if working on top of
> next, at least.
> 
> Is it a good idea to send a patch with all of these things removed or
> they might be coming back later and so they should be kept?
> 
> Thanks in advance.

So in summary you found some unused code and deleted it?  Please send
that patch.

When I'm reviewing "delete unused code" patches, what I'm basically
looking at are the + lines in the commit.  If there are no plus lines
and we're deleting whole functions and macros and the code still
compiles fine then that's easy to review.

When you're sending patches please consider how to organize and explain
them so they are easy to review.

regards,
dan carpenter




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux