Fix a crash when dragging a debug conhost across a DPI boundary

[zadjii-msft]

Summary of the Pull Request

When dragging DEBUG conhost across a DPI boundary, we'd crash. This doesn't repro for some reason on Release builds. Maybe @miniksa can share some light why that is.

PR Checklist

• Closes Dragging conhost across a DPI boundary crashes it (Debug only 🤞) #4012
• I work here
• Tests added/passed
• [n/a] Requires documentation to be updated

Validation Steps Performed

Dragged it across the boundary again, doesn't crash anymore 🙏

[ghost]

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 7 hours 37 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[miniksa]

This change doesn't show understanding of what actually was failing and has no tests to prevent it from happening again.

I'd at minimum want a test or some change that enforces that FontInfo cannot accept a nullptr if it's causing crashes.

But more certainly, I'd want whatever crashed to be guarded and to understand what type of crash it actually was.

[zadjii-msft]

@miniksa The crash seemed to be related to passing a nullptr to the wstring_view ctor. This post seems to suggest that you can't do that. I imagine that audit mode will check this for other callsites, though I don't know enough to be certain.

I did check all the FontInfo instantiations to check if any others are passing in nullptr, and they seemed fine.

[miniksa]

@miniksa Michael Niksa FTE The crash seemed to be related to passing a nullptr to the wstring_view ctor. This post seems to suggest that you can't do that. I imagine that audit mode will check this for other callsites, though I don't know enough to be certain.

I did check all the FontInfo instantiations to check if any others are passing in nullptr, and they seemed fine.

Can you change the FontInfo constructors to assert or fail if someone tries to put a nullptr in there again at least?

I just don't like that there's nothing stopping this from happening again by accident. I'm not certain audit would catch this.

[zadjii-msft]

Can you change the FontInfo constructors to assert or fail if someone tries to put a nullptr in there again at least?

How would I actually do that? The current FontInfo ctor is the following:

    FontInfo(const std::wstring_view faceName,
             const unsigned char family,
             const unsigned int weight,
             const COORD coordSize,
             const unsigned int codePage,
             const bool fSetDefaultRasterFont = false);

Would I just define a FontInfo(std::nullptr_t* facename, ...) that throws? Presumably then, passing in nullptr would use that ctor, and not the wstring_view one.

I guess my next question though would be: do we need to do this everywhere we're accepting string_view's?

[miniksa]

Nnnnnnn. I guess that it isn't solvable then because it comes out of the constructor. I was imagining that it was accepting the nullptr and then making the string/view inside itself so you could E_INVALIDARG it.

I'll stop blocking then.