-
Notifications
You must be signed in to change notification settings - Fork 165
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
Fix profiling when IO_Fork from the IO package is used #2908
Conversation
7fafc33
to
29b3368
Compare
src/profile.c
Outdated
{ | ||
HashLock(&profileState); | ||
if (profileState_Active) { | ||
char filenamecpy[GAP_PATH_MAX + 20]; |
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.
This is not a blocking comment, but still: Why the +20? That seems to defeat the purpose of GAP_PATH_MAX
. If we are concerned the buffer might not be big enough (a quite reasonable concern, although hopefully in practice rarely of relevance), well, then we should avoid hard coded buffer sizes at all everywhere. OTOH, if we accept that we'll only support paths up to a certain length, then why suddenly make an exception here?
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.
It's because otherwise I get a warning from gcc complaining my snprintf could overflow it's buffer (which seems like a stupid warning, as isn't that the point of snprintf?) I couldn't figure out how to disable.
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.
However, I accept that's stupid. I've now done it "properly", without having any warnings.
29b3368
to
93530ca
Compare
Codecov Report
@@ Coverage Diff @@
## master #2908 +/- ##
==========================================
- Coverage 83.74% 83.74% -0.01%
==========================================
Files 681 680 -1
Lines 346547 346275 -272
==========================================
- Hits 290228 289973 -255
+ Misses 56319 56302 -17
|
Many apologises for the multiple PRs.
After further thought #2907 is both (a) too complicated, and (b) doesn't even do what I needed (it makes too many files). This adds a tiny part of that PR. I will then make a seperate PR to IO, which calls this function when in the child part of an IO_fork.
While this will tie profiling and IO_fork more tightly, I think that's the right option, as the forks in GAP never make processes which can continue to run GAP.