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

Would it be reasonable for functions that accept text_begin/text_end to allow text_begin equals text_end equals NULL? #7391

Closed
alien-brother opened this issue Mar 11, 2024 · 4 comments

Comments

@alien-brother
Copy link

Version/Branch of Dear ImGui:

Version 1.90.4, Branch: master (I guess?)

Back-ends:

(Unrelated to backends) imgui_impl_win32.cpp + imgui_impl_dx11.cpp

Compiler, OS:

(Unrelated to compiler and OS) MSVC 2022, Windows 11

Full config/build information:

Unrelated to configuration

Details:

I'm looking at ImDrawList::AddText(), but there are other functions that accept a text range. The code that handles text_begin/text_end looks like this:

    if (text_end == NULL)
        text_end = text_begin + strlen(text_begin);
    if (text_begin == text_end)
        return;

If text_begin equals text_end equals NULL, the code will call strlen() on NULL. This can reasonably happen when text_begin and text_end are directly taken from an std::string_view as data() and data() + size(), since a default-constructed std::string_view has NULL as data() as size() of 0. I believe, adding 0 to a null pointer is valid.

Instead, AddText (and other functions) could do something like

    if (text_end == NULL && text_begin != NULL)
        text_end = text_begin + strlen(text_begin);
    if (text_begin == text_end)
        return;

Obviously, the caller can work around this by placing the condition text_begin != NULL before the call.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

std::string_view empty_view;
ImGui::GetWindowDrawList()->AddText(ImGui::GetCursorScreenPos(), IM_COL32(0, 0, 0, 0), empty_view.data(), empty_view.data() + empty_view.size());
@ocornut
Copy link
Owner

ocornut commented Mar 12, 2024

Yes, in this situation I think we can support it, same as #3615

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Mar 27, 2024
@ocornut
Copy link
Owner

ocornut commented Mar 27, 2024

I have pushed the fix for this 37b37fc
And the test: ocornut/imgui_test_engine@a3b0296

Thank you!

@ocornut ocornut closed this as completed Mar 27, 2024
@alien-brother
Copy link
Author

Thanks. This helps get rid of some boilerplate code.

@alien-brother
Copy link
Author

Another existing API that does not allow text_begin == text_end == NULL is ImFont::CalcTextSizeA().
https://github.com/ocornut/imgui/blob/master/imgui_draw.cpp#L3915

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

No branches or pull requests

2 participants