Created attachment 501235 [details] [diff] [review] Rename duplicates There are sum functions declared with duplicated parameters name. That is not valid c++. See "Parameter name lookup" in http://clang.llvm.org/compatibility.html#c++
The reporter's summary and initial comment were both lame. I'm merely adjusting the summary and providing a better link. I am not passing judgement on the quality of the bug report. http://clang.llvm.org/compatibility.html#param_name_lookup
Summary: Duplicated parameter name → exception_raise_state exception_raise_state_identity have Duplicated parameter name thread_state_count
Comment on attachment 501235 [details] [diff] [review] Rename duplicates one's an out variable, either they should use a polish notation or an out indicator, this is the wrong fix.
Attachment #501235 - Flags: review?(ted.mielczarek) → review-
A few samples found from Googling name them old_state and new_state: http://www.brianweb.net/misc/mach_exceptions_demo.c http://www.cs.cmu.edu/~cache/mach/src/export-osfmach3/osfmach3_ppc/include/mach/exc_server.h.html but the Apple man page names them in_state and out_state: http://www.opensource.apple.com/source/xnu/xnu-792.13.8/osfmk/man/catch_exception_raise.html either way we need to upstream this patch: http://code.google.com/p/google-breakpad/
Sorry, I am not sure that I follow. This is not a style question. The code is invalid c++. I will upstream the patch if it is already not there, but really need to rename of the arguments.
He's saying (and I agree), that your chosen replacement names kind of suck, that's all. I can land these patches upstream for you once we get them all reviewed.
This is now http://breakpad.appspot.com/248001
Created attachment 501326 [details] [diff] [review] copy of the patch sent upsteram
Attachment #501326 - Flags: review?
Attachment #501326 - Flags: review? → review?(ted.mielczarek)
Comment on attachment 501326 [details] [diff] [review] copy of the patch sent upsteram r+ed upstream as well. I'll coordinate landing all these in a bit.
Attachment #501326 - Flags: review?(ted.mielczarek) → review+
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Landed upstream: http://code.google.com/p/google-breakpad/source/detail?r=765 Rafael, can you close the Breakpad review issue? http://breakpad.appspot.com/248001/show
You need to log in before you can comment on or make changes to this bug.