Re: [RFC 5/5] rtl8192u: fix identation in r8192U_core.c

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

 



On 06/01/2013 01:47 AM, Dan Carpenter wrote:
On Fri, May 31, 2013 at 08:10:52PM +0300, Xenia Ragiadakou wrote:
This patch fixes identation and alignment in r8192U_core.c.
Also, removes spaces from idents when applicable.

Signed-off-by: Xenia Ragiadakou<burzalodowa@xxxxxxxxx>
---

Please take a look at changes in stage:
@@-2686,35 +2688,35 @@ static void rtl8192_read_eeprom_info(struct net_device *dev)
There are some changes that i don't know where they
came from. They do not alter the code though.
I am referring to the following sub-add pairs:
patch lines 922 and 923
patch lines 925 and 926
patch lines 928 and 945

I am worried about this but I don't understand it.  What do those
line numbers mean?  Please explain again.

They are the lines in the patch file where I saw the changes when
I was reviewing the patch.

Don't put RFC.  It's sort of cowardly.  Be fearless!

I do not revert never ever again!


patch 2/2 will have to be redone.  so my guess is that 3-5 won't
apply after 2/2 is redone.

Patches 1, 3 and 4 are great.  Patch 5 is huge but I don't see a
clear way to break it into smaller patches, so I guess it's fine.
I have scripts to review that kind of patch so it's not a problem.

@@ -236,12 +236,12 @@ static void rtl819x_set_channel_map(u8 channel_plan, struct r8192_priv *priv)
  }


-#define		rx_hal_is_cck_rate(_pdrvinfo)\
-			(_pdrvinfo->RxRate == DESC90_RATE1M ||\
-			_pdrvinfo->RxRate == DESC90_RATE2M ||\
-			_pdrvinfo->RxRate == DESC90_RATE5_5M ||\
-			_pdrvinfo->RxRate == DESC90_RATE11M)&&\
-			!_pdrvinfo->RxHT\
+#define rx_hal_is_cck_rate(_pdrvinfo)			\
+	(_pdrvinfo->RxRate == DESC90_RATE1M ||		\
+	 _pdrvinfo->RxRate == DESC90_RATE2M ||		\
+	 _pdrvinfo->RxRate == DESC90_RATE5_5M ||	\
+	 _pdrvinfo->RxRate == DESC90_RATE11M)&&	\
+	!_pdrvinfo->RxHT				\

This macro is disgusting.  It should be a function.

bool rx_hal_is_cck_rate(struct rx_drvinfo_819x_usb *pdrvinfo)
{
	if (pdrvinfo->RxHT)
		return false;

	switch (pdrvinfo->RxRate) {
	case DESC90_RATE1M:
	case DESC90_RATE2M:
	case DESC90_RATE5_5M:
	case DESC90_RATE11M:
		return true;
	default:
		return false;
	}
}

regards,
dan carpenter

To be honest i don't know the comparative advantages
of the two styles (not the visual ones).

(irrelevant)
I have a problem with ifdefed code. I am not sure as far
as which ifdefs should be removed completely.
Even the TODO and debug ifdefs appear useful to me
if I was a developer that works on this driver.
Is this the right thing to delete them?

Thanks,
Ksenia


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel




[Index of Archives]     [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