Skip to content

Conversation

@gSahitya-samsung
Copy link
Contributor

@gSahitya-samsung gSahitya-samsung commented Dec 17, 2024

Issue: Getting below build warnings

include/signal.h:590:83: WARNING: 'int sigaction(int, const sigaction*, sigaction*)' hides constructor for 'struct sigaction' [-Wshadow]

  590 | int sigaction(int sig, FAR const struct sigaction *act, FAR struct sigaction *oact);

include/stdlib.h:603:30: WARNING: 'mallinfo mallinfo()' hides constructor for 'struct mallinfo' [-Wshadow]

  603 | struct mallinfo mallinfo(void);

Cause: the struct sigaction and function sigaction is declared with same
name. During compilation they were part of same namespace that's
causing shadowing of struct with function declaration. Same is true for
mallinfo

Resolution: Disable shadow warning during function declaration and
enable it after declaration.

Note: GCC>=7 doesn't throw this error with -Wshadow flag. This
temporary changes is only needed to support build with old version of
GCC.

sunghan-chang
sunghan-chang previously approved these changes Dec 17, 2024
Copy link
Contributor

@ritesh55555 ritesh55555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build warnings resolved

Copy link
Contributor

@CookieDoughMixer CookieDoughMixer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a work around. Is it possible that these errors are occuring because we include c headers in cpp files? Will it be possible to resolve them by adding extern C at appropriate places?

@gSahitya-samsung
Copy link
Contributor Author

This looks like a work around. Is it possible that these errors are occuring because we include c headers in cpp files? Will it be possible to resolve them by adding extern C at appropriate places?

Yes, its work around

The proper fix can be as follow:

  1. Either change struct or function name
  2. Move functions declaration into separate header file

But in our case, none of above will work.

Can we change -Wshadow to -Wshadow=compatible-local? It will also resolve the warnings message.

Reference - https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

@r-prabu
Copy link
Collaborator

r-prabu commented Dec 17, 2024

This looks like a work around. Is it possible that these errors are occuring because we include c headers in cpp files? Will it be possible to resolve them by adding extern C at appropriate places?

@abhishek-samsung i think it is not because of cpp files. Also this is applied only for the few required lines which should not create a problem.

r-prabu
r-prabu previously approved these changes Dec 17, 2024
@kishore-sn
Copy link
Contributor

Can we change -Wshadow to -Wshadow=compatible-local? It will also resolve the warnings message.

I think this might be a better option. Please try it out.

Issue: Getting below build warnings

```
include/signal.h:590:83: WARNING: 'int sigaction(int, const sigaction*, sigaction*)' hides constructor for 'struct sigaction' [-Wshadow]

  590 | int sigaction(int sig, FAR const struct sigaction *act, FAR struct sigaction *oact);

include/stdlib.h:603:30: WARNING: 'mallinfo mallinfo()' hides constructor for 'struct mallinfo' [-Wshadow]

  603 | struct mallinfo mallinfo(void);

```

Cause: the struct `sigaction` and function `sigaction` is declared with same
name. During compilation they were part of same  namespace that's
causing shadowing of struct with function declaration. Same is true for
`mallinfo`

Resolution: Disable shadow warning during function declaration and
enable it after declaration.

Note: GCC>=7 doesn't throw this error with -Wshadow flag. This
temporary changes is only needed to support build with old version of
GCC.
@kishore-sn kishore-sn merged commit 42ac189 into Samsung:master Dec 19, 2024
11 checks passed
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.

6 participants