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

Repair header file include guards #2623

Closed
haukepetersen opened this issue Mar 17, 2015 · 17 comments · Fixed by #2675, #2676, #2677, #2678 or #2679
Closed

Repair header file include guards #2623

haukepetersen opened this issue Mar 17, 2015 · 17 comments · Fixed by #2675, #2676, #2677, #2678 or #2679
Assignees
Labels
Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented

Comments

@haukepetersen
Copy link
Contributor

As we were pointed out a while ago, defines that start with underscores are not allowed by the C standard (I forgot the PR number where we discussed this -> could somebody help here?).

So the following style

#ifndef __MSG_H_
#define __MSG_H_

...

#endif /* __MSG_H_ */

would need to be changed to something like

#ifndef MSG_H
#define MSG_H
...
#endif /* MSG_H */

Git grep tells me there are a lot of files that need to be adjusted. So if anyone is taking this on, please do it in small, easily reviewable and logical grouped chunks...

@jnohlgard
Copy link
Member

Use the following command to find the offending include guards:

git grep -E 'ifndef\s*__'

@OlegHahm OlegHahm assigned ghost and unassigned OlegHahm Mar 17, 2015
@BigDaddyD
Copy link
Contributor

So writing a script for this change is out of the question?

@jnohlgard
Copy link
Member

You may choose whatever method you like, a script is fine as long as it does the right thing. We will still need to do a manual code review for the change if/when you open a PR.

@kushalsingh007
Copy link
Member

working on this one :)

@shrenikjain38
Copy link

Do we have to edit the defines starting with single underscores as well?
And by defines is it meant that it has to be done for #define and #ifdef as well?

@jnohlgard
Copy link
Member

@shrenikjain38 Changing _FOO is a good idea. Identifiers beginning in single underscore followed by a upper case letter is also reserved.

btw, your scripted commit https://github.com/shrenikjain38/RIOT/commit/fd40ba441aa3ac6e1223d14ba275b5ee15d5e5eb breaks the C++ checks (don't modify #ifdef __cplusplus)

The #ifdef HEADER_FILE_NAME_H_ and #define HEADER_FILE_NAME_H_ are include guards that protect against multiple inclusion of the same file, both #define and #ifdef must be changed together.
Also, don't forget that there is usually a comment at the #endif containing the same symbol name as well which should be updated for consistency. e.g. #endif /* HEADER_FILE_NAME_H_ */

@jnohlgard
Copy link
Member

another hint for anyone preparing pull requests for this issue: Read the description at the top, we won't be able to merge the changes if you don't split them into small logical groups, we need to be able to review the changes manually before we merge them.

So if anyone is taking this on, please to it in small, easily reviewable and logical grouped chunks...

@shrenikjain38
Copy link

Changing _FOO is a good idea. Identifiers beginning in single underscore followed by a upper case letter is also reserved.

Can you please elaborate it a bit more if the identifiers beginning with a single underscore are to be changed and if they are, whether those followed by an upper case letter after an underscore to be modified or not?

Also, don't forget that there is usually a comment at the #endif containing the same symbol name as well which should be updated for consistency. e.g. #endif /* HEADER_FILE_NAME_H_ */

This issue has already been taken into consideration.

Is it okay if the changes are made in a specific directory to divide the whole procedure in easily reviewable and logically grouped chunks?

Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
haukepetersen added a commit that referenced this issue Mar 22, 2015
Repair header file include guards #2623
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
Darredevil added a commit to Darredevil/RIOT that referenced this issue Mar 22, 2015
@BigDaddyD
Copy link
Contributor

I am currently working on fixing all headers under the boards directory. I managed to use git grep and sed to find and fix all ifndefs and defines. I was wondering if that can be used to fix the comments as well

@PeterKietzmann
Copy link
Member

@BigDaddyD what's the status? Are you still working on it?

@miri64
Copy link
Member

miri64 commented Oct 17, 2016

There are still some of these macros left. In sys in particular.

@zhuoshuguo
Copy link
Contributor

@miri64 Will find time to handle.

@miri64
Copy link
Member

miri64 commented Jan 20, 2017

Some are still left by git's non-concurrency ;-):

[mlenders@mk RIOT]<3 git grep "H_$"
boards/nucleo32-f303/include/board.h:#ifndef BOARD_H_
boards/nucleo32-f303/include/board.h:#define BOARD_H_
boards/nucleo32-f303/include/periph_conf.h:#ifndef PERIPH_CONF_H_
boards/nucleo32-f303/include/periph_conf.h:#define PERIPH_CONF_H_
cpu/k60/include/MK60D10.h:#define MK60D10_H_
cpu/k64f/include/MK64F12.h:#define MK64F12_H_
cpu/kw2x/include/MKW22D5.h:#define MK22D5_H_
cpu/native/include/netdev2_tap_params.h:#ifndef NETDEV2_TAP_PARAMS_H_
cpu/native/include/netdev2_tap_params.h:#define NETDEV2_TAP_PARAMS_H_
examples/ccn-lite-relay/Makefile:CFLAGS += -DCCNL_UAPI_H_
Binary file pkg/emb6/patches/0002-Rename-colliding-files-and-functions.patch matches
Binary file pkg/oonf_api/patches/0001-add-RIOT-support.patch matches
Binary file pkg/oonf_api/patches/0009-Add-missing-include.patch matches
sys/include/uuid.h:#ifndef UUID_H_
sys/include/uuid.h:#define UUID_H_

@miri64 miri64 added the Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment