lwfinger / rtw88

A backport of the Realtek Wifi 5 drivers from the wireless-next repo.
647 stars 183 forks source link

Support old C99 Kernel Compile #257

Closed briansune closed 3 days ago

briansune commented 3 days ago

The old C syntax does not allow loop initialization variable type for(int i=0 .... into int i; for (i=0

a5a5aa555oo commented 3 days ago

Hi,

Do you encounter any problem when building this driver? doesn't this PR fix this?

briansune commented 3 days ago

@a5a5aa555oo

If not consider the led marco issue that mentioned here.

@dubhater

After pull the repository I found there is a dependence issue related to

#ifdef CONFIG_MAC80211_LEDS bool led_registered; char led_name[32]; struct led_classdev led_cdev; #endif

It will return errors:

kernel.h:994:51: error: 'struct rtw_dev' has no member named 'led_cdev' 994 | BUILD_BUG_ON_MSG(!same_type((ptr), ((type )0)->member) && compiler_types.h:129:35: error: 'struct rtw_dev' has no member named 'led_cdev' 129 | #define compiler_offsetof(a, b) __builtin_offsetof(a, b) main.c:2373:36: error: 'struct rtw_dev' has no member named 'led_cdev' 2373 | struct led_classdev *led = &rtwdev->led_cdev; main.c:2384:17: error: 'struct rtw_dev' has no member named 'led_name' 2384 | snprintf(rtwdev->led_name, sizeof(rtwdev->led_name)

After modifying the C99 standard style it can compile normally.

I only see the only part during compile of the entire repository w/o the flag of:

-std=gnu11 -Wno-declaration-after-statement

are those two files C99 syntax issues. after manually modify it will. Compile normally with no issue.

briansune commented 3 days ago

@a5a5aa555oo

The reason why I would suggest this pull request is because Makefile is a frequently modified file. In my case I clone from old RTW88 makefile and modified to cross-compile the driver. But I didn't notice there are flags added during the RTW88 driver is updated. So turns on you see this PR again.

So if you ask me I don't think adding flags is a good practice as old coding style always make no issue to new compiler.

a5a5aa555oo commented 3 days ago

@a5a5aa555oo

The reason why I would suggest this pull request is because Makefile is a frequently modified file. In my case I clone from old RTW88 makefile and modified to cross-compile the driver. But I didn't notice there are flags added during the RTW88 driver is updated. So turns on you see this PR again.

So if you ask me I don't think adding flags is a good practice as old coding style always make no issue to new compiler.

Yes, totally agree, but this kind of coding style will appear more often because it is allowed by kernel.

briansune commented 3 days ago

@a5a5aa555oo

I am not really a coding guy but with my limited study on C. The reason behind the variable MUST initialized on the top of the function is because optimization is quicker. And people who knows coding better old days always use as less resources as possible so this make so much sense. But newer days like now people don't even need to care as compiler take cares all this messy coding and compile it /w no problem.

So it is allowed v.s. it is good to write in such way is a big against. Why write

for (int i

than only write a single variable on the top and reuse always in each clean loop. you do need to type more "int" in the coding =] Right? And then you need to use additional flag to clear it out on supporting different revisions. Just because it is a really small problem from beginning. This is why regulation in coding is very important from beginning.