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
My favorite is ucb/source/ucp/file/filerror.hxx, which has the #ifndef, but not the #define...
Comment on attachment 85069 [details] Header files with missing guard Mimetype fixed
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
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.
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."
Thanks Stephan, I'll take care of it.
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.
(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.
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.
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.
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.)
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
Thanks, Carl. Great work! :)
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner TopicCleanup) [NinjaEdit]
Remove LibreOffice Dev List from CC on EasyHacks (curtailing excessive email to list) [NinjaEdit]