CurrentThreadId does not have a return for an unreachable exit path (missing return)

RESOLVED FIXED in mozilla2.0b10

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

Trunk
mozilla2.0b10
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch patch (obsolete) — Splinter Review
CurrentThreadId is missing a returns which causes clang to produce a warning that because of -Werror=return-type causes the build to fail.
Attachment #501239 - Attachment is patch: true
Attachment #501239 - Attachment mime type: application/octet-stream → text/plain
Attachment #501239 - Flags: review?(ted.mielczarek)

Comment 1

9 years ago
1959 CurrentThreadId()
1960 {
1961 #if defined(XP_WIN)
1962   return ::GetCurrentThreadId();
1963 #elif defined(XP_LINUX)
1964   return sys_gettid();
1965 #elif defined(XP_MACOSX)
1966   // Just return an index, since Mach ports can't be directly serialized
1967   thread_act_port_array_t   threads_for_task;
1968   mach_msg_type_number_t    thread_count;
1969 
1970   if (task_threads(mach_task_self(), &threads_for_task, &thread_count))
1971     return -1;
1972 
1973   for (unsigned int i = 0; i < thread_count; ++i) {
1974     if (threads_for_task[i] == mach_thread_self())
1975       return i;
1976   }
1977 #else
1978 #  error "Unsupported platform"
1979 #endif
1980 }

It's tempting to suggest abort() is better than return -1.
Summary: missing return → CurrentThreadId does not have a return for an unreachable exit path (missing return)
It kind of sucks either way, certainly, but these functions are incredibly unlikely to fail, so maybe an abort makes sense.
Rafael: FWIW, given how small all these patches are, I would have been fine with them all on one bug, like "fix CLang compilation issues in Breakpad".
Assignee: nobody → respindola
Attachment #501414 - Flags: review?(ted.mielczarek)
Attachment #501414 - Flags: review?(ted.mielczarek) → review+

Updated

9 years ago
Attachment #501414 - Flags: approval2.0?

Updated

9 years ago
Attachment #501414 - Flags: approval2.0? → approval2.0+

Comment 5

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b17f28507ab4
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.