Closed
Bug 470282
Opened 16 years ago
Closed 12 years ago
nsProxyRelease.h include guard is not setup properly
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: dbradley, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
538 bytes,
text/plain
|
Details | |
3.57 KB,
patch
|
mozilla+ben
:
review+
|
Details | Diff | Splinter Review |
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?
Comment 1•15 years ago
|
||
(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.
Updated•15 years ago
|
Attachment #366947 -
Attachment mime type: application/octet-stream → text/plain
Comment 2•15 years ago
|
||
Proper bugs, with their containing files.
Attachment #366947 -
Attachment is obsolete: true
Updated•15 years ago
|
Attachment #366948 -
Attachment mime type: application/octet-stream → text/plain
Comment 3•15 years ago
|
||
Easy!
Comment 4•15 years ago
|
||
@Ted - Low priority, but maybe we could turn this into a pre-commit hook?
Comment 5•15 years ago
|
||
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+
Comment 6•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/75658e81e628
Comment 7•15 years ago
|
||
(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 :)
Comment 9•15 years ago
|
||
(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
Comment 10•12 years ago
|
||
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.
Description
•