Closed Bug 470282 Opened 16 years ago Closed 12 years ago

nsProxyRelease.h include guard is not setup properly

Categories

(Core :: XPCOM, defect)

defect
Not set
minor

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dbradley, Unassigned)

Details

Attachments

(2 files, 1 obsolete file)

One label has one more underscore at the end than does the other

39 #ifndef nsProxyRelease_h_
40 #define nsProxyRelease_h__

Do I really need to submit a patch to chop or add a single underscore?
Attached file output of my search command (obsolete) —
(In reply to comment #0)
> Do I really need to submit a patch to chop or add a single underscore?

Apparently you do (or someone does), since the problem still exists!

Noticed this a couple of days ago and thought it might be a common mistake, so I ran a shell command to look for obvious instances of it:

    find . -name \*.h -exec \
        grep --max-count=1 --after-context=1 "#ifndef \w\+_h" | \
        uniq -u -f 1 | \
        grep --before-context=1 "#define"

That is, search all .h files for the first #ifndef followed by a \w+_h identifier, print the matching lines with one extra line of context to capture the subsequent #define, print all distinct pairs (ignoring the #ifndef and #define parts), and filter the resulting pairs to just those in which there was actually a #define on the second line.

This search produced false positives, as you might imagine, but the resulting list is small enough to be manually verifiable.
Attachment #366947 - Attachment mime type: application/octet-stream → text/plain
Proper bugs, with their containing files.
Attachment #366947 - Attachment is obsolete: true
Attachment #366948 - Attachment mime type: application/octet-stream → text/plain
@Ted - Low priority, but maybe we could turn this into a pre-commit hook?
Comment on attachment 366952 [details] [diff] [review]
obvious fixes

Builds on all platforms:

https://build.mozilla.org/tryserver-builds/2009-03-11_17:31-bnewman@mozilla.com-try-4560d102332/

Seems like an adequate vetting of a preprocessor-only patch, so I'ma just r+ this patch myself.
Attachment #366952 - Flags: review+
(In reply to comment #4)
> @Ted - Low priority, but maybe we could turn this into a pre-commit hook?

I dunno, is it really something that happens all the time? I mean, we could have a pre-commit hook for everything that you could possibly do wrong, but it would get pretty expensive!
given how many header files we have and how few you had to fix, i'd say it's rare and not really worth it.

but thanks for fixing this :)
(In reply to comment #7)
> I dunno, is it really something that happens all the time? I mean, we could
> have a pre-commit hook for everything that you could possibly do wrong, but it
> would get pretty expensive!

(In reply to comment #8)
> given how many header files we have and how few you had to fix, i'd say it's
> rare and not really worth it.

@ted, @timeless: agreed
Include guards in the attached patch are currently either okay or from files that don't seem to exist any more.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: