-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 put_time()
crash on invalid format specifier
#4840
Fix put_time()
crash on invalid format specifier
#4840
Conversation
feb7f77
to
ee36e11
Compare
3938d20
to
66ee94f
Compare
66ee94f
to
c316691
Compare
put_time()
crash on invalid format specifier
@StephanTLavavej Sorry for the ping, but I see that not only this, but quite few more PRs are waiting for reviews and eventual merge for considerable time. I'm wondering what is the workflow in general? How much time it usually takes and do you have dedicated time for reviewing community supplied PRs? |
My apologies for the delay - yes, the backlog has been accumulating. (This is something that I monitor with the STL Status Chart.) The problem is that I am currently the only STL maintainer able to spend up to 100% of their time on the STL. (See our Maintainer Priorities issue #4700 for weekly updates.) Everyone else is working on high-priority projects like ASAN and even requesting small amounts of their time to review my own PRs is difficult (but necessary as all code must be reviewed by at least one other person, not even I am allowed to check in whatever I want). I occasionally have non-STL tasks to deal with (ConcRT, ATL/MFC every other quarter). Additionally, I just took a few days off for vacation (my niece's cat-themed 6th birthday party), during which the backlog further accumulated, and as soon as I got back I was slammed with high-priority STL-specific but non-review work - I had to fix a regression on Win11 24H2 (#4844), revert my glorious changes to drop Win7 (#4857), update our LLVM submodule to fix internal "credential scanner" failures (#4862), and fix When the world isn't on fire and I can sit down and review PRs, usually I can dedicate most of my day to it (excluding only meetings and email). The time that it takes to review a PR varies, depending on how complicated the problem and solution are, how familiar I am with the area, how well-structured the PR's commit history and description are, and how much the PR has already been reviewed by other contributors. If the PR is clean and I know the area deeply, I can easily power through a thousand lines in an hour. If the area is complicated, or I notice that something is messed up, a few dozen lines can take longer to either fix or write up what's wrong. When there are no major distractions, I can usually keep up with or slightly exceed the rate of incoming PRs, which is how we've been able to drain the formerly-numbering-80 PR backlog. But as queueing theory explains, when a source and drain are closely balanced with little excess capacity, any increase in source throughput or reduction in drain throughput causes a massive increase in the backlog (aka why store checkout lines can be massive, etc.). I'm hoping to make review progress next week, although I can't promise anything. (I try to review high-priority PRs first, but the order in which I review PRs is highly variable - I also like to review easy PRs first, until I'm in the zone and can really focus on complicated changes.) |
C++ currently refers to C17, but C23 lists the same specifiers and fixes a damaged mention of %b.
I can see, why PR reviews can take so much time. You not only do the reviews, but also fix your own comments. :) |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thanks for fixing this longstanding bug with severe impact, and congratulations on your first microsoft/STL commit! 😻 🛠️ 🥳 This is expected to ship in VS 2022 17.12 Preview 3. |
Thank you! What is the dead line to catch the 17.12 RTM version as I plan to have two more PR's regarding P.S. Also, just out of curiosity, why there is an 'internal repo' on which you mirror the PRs? |
Changes will stop flowing from our development branch into 17.12 Preview 3 on 2024-08-30 (3 weeks from now). That's not the release date, it's just when an internal merge from Aug 30 is the date that the branch is "snapped" but we need at least a couple of days to review and merge a change, so I'd say that a PR has to be completely ready by Mon, Aug 26 if it's to have a chance of making it through the review and merge process in time. The internal repo is where all of the MSVC toolset (including the compiler front-end, back-end, linker, and other libraries) is built. While 99.9% of STL development happens on GitHub and is then mirrored to MSVC, the MSVC-internal repo builds more flavors that we haven't ported to our CMake build system yet (notably Although all of that internal stuff is happening, we ensure that our GitHub repo is binary-identical at nearly all times to a subdirectory of our internal repo ( |
Thank you for the detailed explanations! I will try my best to catch the deadline. |
Fixes #4820. The test case also verifies the situation where an invalid specifier is given when a modifier is being used. For example "%E% some text"
This is my first PR, so sorry if something is wrong in formatting or ABI terms.