Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues that trigger compiler warnings with gcc 7.2.1 #7

Open
wants to merge 1 commit into
base: aosp/LA.UM.5.7.r1
Choose a base branch
from

Conversation

berolinux
Copy link

Small issues: Misleading indentation looking like an if statement
ends in the wrong place, checking a variable of type bool for
values > true (impossible), etc.

The only change in here that has any effect other than silencing
warnings and reducing the headaches of someone trying to read
the code is changing the type of enablePowersaveOffload in
struct hdd_config: According to the code in wlan_hdd_main.c, it is
clearly meant to take 3 different values (0/nodeepsleep/deepsleep), so
it can't be bool.
Another option may be to keep it as bool and unify 0 and nodeepsleep
into false and changing deepsleep to true

Signed-off-by: Bernhard Rosenkränzer bero@lindev.ch

Small issues: Misleading indentation looking like an if statement
ends in the wrong place, checking a variable of type bool for
values > true (impossible), etc.

The only change in here that has any effect other than silencing
warnings and reducing the headaches of someone trying to read
the code is changing the type of enablePowersaveOffload in
struct hdd_config: According to the code in wlan_hdd_main.c, it is
clearly meant to take 3 different values (0/nodeepsleep/deepsleep), so
it can't be bool.
Another option may be to keep it as bool and unify 0 and nodeepsleep
into false and changing deepsleep to true

Signed-off-by: Bernhard Rosenkränzer <bero@lindev.ch>
@jerpelea
Copy link
Collaborator

jerpelea commented Jan 23, 2018

please do not add/ remove spaces because it will make things harder on the next rebase.

@berolinux
Copy link
Author

berolinux commented Jan 23, 2018

Adding/changing the spaces is necessary to make the warnings (which the kernel's compiler wrapper script turns into errors) go away -- the indentation in those files is broken.
Here's a short snippet showing why this is necessary:

#include <stdio.h>
int main(int argc, char **argv) {
        if(1)
                puts("Test 1");
                puts("Test 2");
}
$ gcc -Wall -Werror test.c
test.c: In function ‘main’:
test.c:3:2: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
  if(1)
  ^~
test.c:5:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
   puts("Test 2");
   ^~~~
cc1: all warnings being treated as errors

The other fix would be allowing those warnings in the wrapper script, but that would never be accepted in the upstream kernel.

@berolinux
Copy link
Author

Where is the upstream for this code? Fixing the indentation issue there is preferable because it wouldn't cause problems during next rebase...

@jerpelea
Copy link
Collaborator

the upstream is in code aurora and there is no way to submit code there

@jerpelea
Copy link
Collaborator

please split the pacth is 2 separate patches
1 indentation
2 boolean fixes
this will ease the rebase process

@jerpelea
Copy link
Collaborator

please update you PR so that I can merge it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants