Skip to content

Simplify a copying the fill from basic_specs#4290

Merged
vitaut merged 1 commit intofmtlib:masterfrom
phprus:issue-4289
Jan 5, 2025
Merged

Simplify a copying the fill from basic_specs#4290
vitaut merged 1 commit intofmtlib:masterfrom
phprus:issue-4289

Conversation

@phprus
Copy link
Copy Markdown
Contributor

@phprus phprus commented Jan 4, 2025

Simplify code.

Fix for #4289

Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
@phprus
Copy link
Copy Markdown
Contributor Author

phprus commented Jan 4, 2025

@ja11sop , test this PR, please.

@ja11sop
Copy link
Copy Markdown

ja11sop commented Jan 5, 2025

I just ran against this PR and all looks good

@phprus
Copy link
Copy Markdown
Contributor Author

phprus commented Jan 5, 2025

@vitaut please review this PR.

@vitaut
Copy link
Copy Markdown
Contributor

vitaut commented Jan 5, 2025

I wonder why the issue reported in #4289 hasn't been caught by existing tests? It might be worth adding a regression test.

@vitaut vitaut merged commit 2c3a569 into fmtlib:master Jan 5, 2025
@phprus
Copy link
Copy Markdown
Contributor Author

phprus commented Jan 5, 2025

@vitaut

I'm guessing that a previous version of the code might have corrupted memory somewhere earlier.

I don't see the root of case for this error, but I think move the fill copying into the basic_specs class as a member function is a nice simplification of the logic.

The code from PR is free of any possibility of memory corruption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants