Download it now!
Bug 68849 - Other: Add header guards to all include files
Summary: Other: Add header guards to all include files
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.2.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Jens Carl
URL:
Whiteboard: BSA target:4.2.0 target:4.3.0 target:...
Keywords: difficultyBeginner, easyHack, topicCleanup
Depends on:
Blocks:
 
Reported: 2013-09-02 15:45 UTC by Thomas Arnhold
Modified: 2016-02-18 16:37 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Header files with missing guard (10.00 KB, text/plain)
2013-09-02 15:45 UTC, Thomas Arnhold
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Arnhold 2013-09-02 15:45:36 UTC
Created attachment 85069 [details]
Header files with missing guard

Currently there are some header files which don't have any include guard at all.

To identify them:

git grep -A1 '^\s*#ifndef' -- '*.hxx' | grep '\-#define\s' | sed 's/-#define.*//g' | sort -u > with-guards.tmp

find . -name *.hxx | sed 's/\.\///g' | sort -u > all-files.tmp

sort all-files with-guards | uniq -u > missing-guards.log


Currently this is a list of 261 files. There are some false positives, mainly because it's a precompliled header or it has 'pragma once' as include guard. But it should give a clou which files need an include guard.
Operating System: All
Version: 4.2.0.0.alpha0+ Master
Comment 1 David Tardon 2013-09-03 12:59:37 UTC
My favorite is ucb/source/ucp/file/filerror.hxx, which has the #ifndef, but not the #define...
Comment 2 Julien Nabet 2013-09-05 20:42:32 UTC
Comment on attachment 85069 [details]
Header files with missing guard

Mimetype fixed
Comment 3 Björn Michaelsen 2013-10-04 18:46:51 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 4 Commit Notification 2013-10-21 09:22:30 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8fc6905674142c226a117a97a08cf0b24c9d4fc1

fdo#68849 add some header guards



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 5 Stephan Bergmann 2013-10-21 10:41:15 UTC
see comment at <https://gerrit.libreoffice.org/#/c/6364/>:  "Note that identifiers starting with an underscore followed by an uppercase letter (_INCLUDED_ANIMCORE_FACTREG_HXX_ etc.) must not be used in C/C++ code (even if lots of our legacy code violates that). In the future, please use just INLCUDED_ANIMCORE_FACTREG_HXX etc."
Comment 6 Thomas Arnhold 2013-10-21 15:02:55 UTC
Thanks Stephan, I'll take care of it.
Comment 7 Thomas Arnhold 2013-10-21 15:25:29 UTC
I'll use the following scheme for all new or changed header guards:

All uppercase: INCLUDED_%modulename_%filename_%extension

Examples:

basic/source/runtime/rtlproto.hxx -> INCLUDED_BASIC_RTLPROTO_HXX
embeddedobj/source/inc/closepreventer.hxx -> INCLUDED_EMBEDDEDOBJ_CLOSEPREVENTER_HXX
sd/source/ui/inc/dlgass.hxx -> INCLUDED_SD_DLGASS_HXX

With this INCLUDED prefix we will always be able to identify this as include guard. This will be an advantage for future scripting efforts.
Comment 8 Stephan Bergmann 2013-10-21 15:32:12 UTC
(In reply to comment #7)
> I'll use the following scheme for all new or changed header guards:
> 
> All uppercase: INCLUDED_%modulename_%filename_%extension
> 
> Examples:
> 
> basic/source/runtime/rtlproto.hxx -> INCLUDED_BASIC_RTLPROTO_HXX
> embeddedobj/source/inc/closepreventer.hxx ->
> INCLUDED_EMBEDDEDOBJ_CLOSEPREVENTER_HXX
> sd/source/ui/inc/dlgass.hxx -> INCLUDED_SD_DLGASS_HXX
> 
> With this INCLUDED prefix we will always be able to identify this as include
> guard. This will be an advantage for future scripting efforts.

FYI, the convention that I use is to use INCLUDED_FOO_BAR_HXX for a global header include/foo/bar.hxx under include/ and the longer INCLUDED_FOO_SOURCE_BAR_BAZ_HXX for anything else like foo/source/bar/baz.hxx, in an attempt to minimize the chance of clashes.
Comment 9 Commit Notification 2013-10-22 03:44:19 UTC
Thomas Arnhold committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d6b80337fb76e31b2e6a11c18c3ae989767c8118

fdo#68849 add some header guards



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Thomas Arnhold 2013-10-23 10:51:11 UTC
fixincludeguards.sh (commit b009e8fd4fce06d9abae8aaac8750ece7df212e4) spits out a warning "guard not detectable" if no include guard is found in a header file (or some other stuff like #pragma once is found). You could use this to identify files with missing include guards.
Comment 11 Jason Gerlowski 2014-03-07 19:16:04 UTC
I'm interested in working on this EasyHack as a little intro project.

I actually have some changes mocked up that I'd like to push for review.  Is anyone interested/willing to be a reviewer on Gerrit?

(In general, I'm not too sure how to find reviewers for a given bug/commit; if I'm asking this in the wrong place, please let me know where to look.)
Comment 12 Miklos Vajna 2014-03-07 19:20:27 UTC
Just go ahead and push your change for review; even if you don't add anyone to CC, we try to review trivial patches within a few days.
Comment 13 Commit Notification 2014-03-12 08:50:52 UTC
Jason Gerlowski committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a8a195c9e722861dac5d9fce282edd9558c797a7

fdo#68849 Add header guards to bridges/* files.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2014-05-28 07:51:35 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7cfb4f864ab9b61241e2b9ffb645e9a0d4dd1a6d

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2014-05-29 12:40:38 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=49a0d8eedf6758c8bceaf7824945167d669b71ff

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2014-05-29 14:04:10 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=626a4283672be163085b90d2240638eb0126c654

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2014-05-30 06:44:04 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=54b85bf51e42f0e74dfaab40dc0a6923dd1f876b

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2014-05-30 06:45:38 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3ddb928061531a8742565bfa91c0aaa76ef88863

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 19 Commit Notification 2014-05-30 06:45:51 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bcd5da6cb3d24801be4b84b3339a4d77e255a917

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 20 Commit Notification 2014-05-30 06:47:22 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9e3cc6b47444f988a68ca3ddcad779ed0480b00c

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 21 Commit Notification 2014-06-01 16:06:08 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7610a80ad1a47f2401b4d6aca9d4f77e5be7c630

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 22 Commit Notification 2014-06-01 16:09:03 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f3e1f476e9cffb75d0620ab2dcfdc1ea077cd9d3

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2014-06-01 16:09:17 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8f0160a10dd2a5746c531040ffbf638e356f6d5d

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2014-06-01 16:13:58 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=57217e1e9f995ec413e5eecaf171481373a0a8d0

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2014-06-01 16:15:32 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e15732bc96c903b2e85bcc30c32fad2cceda5610

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2014-06-01 16:15:49 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=158672fda5e9d82906972d9168025d42f3b38ac3

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 27 Commit Notification 2014-06-01 16:17:23 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6c3245fc69de59cb1de219203c489d891073ced9

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 28 Commit Notification 2014-06-01 16:17:39 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=dcb32a33a32665a6fa54df224fcfb2efad411567

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2014-06-02 08:47:22 UTC
Jens Carl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8e115c60081b245541408889295bc0c147091e0c

fdo#68849: Add header guards to all include files



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 30 Jens Carl 2014-06-02 17:31:09 UTC
So after the last commit was approved all missing header guards are gone.

There are still some files with a different type of guard like 'pragma once' or 'define' to let the file only include by a special one. You can find them with three commands mentioned in the first comment and filter out the 'precompiled_*.hxx' files and the 'workdir/' directory. 

This left 26 files on my system.

Should I missed one, please re-open this bug.
Comment 31 Thomas Arnhold 2014-06-03 12:19:42 UTC
Thanks, Carl. Great work! :)
Comment 32 Robinson Tryon (qubit) 2015-12-15 12:10:00 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner TopicCleanup)
[NinjaEdit]
Comment 33 Robinson Tryon (qubit) 2016-02-18 16:37:17 UTC
Remove LibreOffice Dev List from CC on EasyHacks
(curtailing excessive email to list)
[NinjaEdit]