-
Notifications
You must be signed in to change notification settings - Fork 997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Demonstrate static checking with curl_handles_lock #2387
base: master
Are you sure you want to change the base?
Conversation
9100ebd
to
0fa0c77
Compare
This can find missing locks at compile-time.
@ggtakec I'd like to move towards static lock checking instead of relying on only dynamic ThreadSanitizer. I think this can find bugs more reliably. What do you think? |
Also the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can promote this amendment.
I have a few questions, so I will summarize them here.
About license
Is there any external license involved in mutex.h?
I was curious about this because when I searched for THREAD_ANNOTATION_ATTRIBUTE__
, I saw some examples of like code.
About AutoLock::Type
This Mutex
class will replace all AutoLock
classes, but can't it implement AutoLock::Type
?
(Is it difficult to add attributes for ThreadSanitizer
?)
However, there is no particular problem even if it cannot be implemented.
@@ -290,8 +293,11 @@ fi | |||
#----------------------------------------------------------- | |||
echo "${PRGNAME} [INFO] Set environment for configure options" | |||
|
|||
echo "CXXFLAGS=${CXXFLAGS}" >> "${GITHUB_ENV}" | |||
echo "CONFIGURE_OPTIONS=${CONFIGURE_OPTIONS}" >> "${GITHUB_ENV}" | |||
cat << EOF > "${GITHUB_ENV}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ${GITHUB_ENV} file has been overwritten.
Please change to addendum.
Is there a reason why you changed to Here Document?
@@ -0,0 +1,150 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure to indent 4 space characters.
Sorry this should be broken into separate PRs:
|
This can find missing locks at compile-time.