Skip to content
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

Remove most uses of StringFromFormat in favor of fmt #11162

Merged
merged 2 commits into from Oct 13, 2022

Conversation

Pokechu22
Copy link
Contributor

StringFromFormat uses printf strings. There are still 2 remaining places where it is used: CubebUtils.cpp (where it's formatting a printf-style message from a library) and HLE_OS.cpp (where it's formatting a printf-style message from games).

@@ -50,7 +52,7 @@ bool CEXIETHERNET::TAPNetworkInterface::Activate()
const int MAX_INTERFACES = 32;
for (int i = 0; i < MAX_INTERFACES; ++i)
{
strncpy(ifr.ifr_name, StringFromFormat("Dolphin%d", i).c_str(), IFNAMSIZ);
strncpy(ifr.ifr_name, fmt::format("Dolphin{}", i).c_str(), IFNAMSIZ);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this isn't done with fmt::format_to? This seems like something that should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format_to and format_to_n never write a null terminator, and I'm not sure if one is needed in this case, so it seemed safer to stick with the original implementation. (Though strncpy also does not write a null terminator if the original string is longer than the buffer size.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine, ifr is zero'd before doing this. Just an observation, since other places do use it; feel free to leave it like that if nobody else has an opinion on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see that earlier zeroing out; I've changed it to format_to_n.

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me otherwise.

Source/Core/DolphinQt/Translation.cpp Outdated Show resolved Hide resolved
Nothing currently uses it. It could theoretically be replaced with fmt support, but I don't think the LOG_VULKAN_ERROR macro is that useful and it'd be better to replace it with regular logging instead.
@AdmiralCurtiss AdmiralCurtiss merged commit 304e1e5 into dolphin-emu:master Oct 13, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants