When deviceName or endPointName are null strlen crashed

[TheKnarf]

This fixes issue #5728.

The problem was that the deviceName was undefined when I tried to create a virtual midi device. That made getName return nil, which in turn crashed lmms when it tried to strlen(nil).

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

• Linux (AppImage): lmms-1.2.3-741%2Bg1322fac-linux-x86_64.AppImage (build link)

Windows

• Windows 32-bit: lmms-1.2.3-741%2Bg1322fac60-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.2.3-741%2Bg1322fac60-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.2-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.2-msvc2017-win64.exe (build link)

macOS

• macOS : lmms-1.2.3-741%2Bg1322fac60-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://10045-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-741%2Bg1322fac-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10045?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://10047-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-741%2Bg1322fac60-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10047?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://10046-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-741%2Bg1322fac60-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10046?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/9lq0tb5b7lt6iab6/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36041367"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/6b1e0exafr60g98i/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36041367"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://10049-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-741%2Bg1322fac60-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10049?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "6fbb0b88026be3cbc4e5ff863aa77c9cf51150c1"}

[TheKnarf]

Seems that circleci failed on the macos build because there's something wrong with the homebrew install? That's not really related to my PR at all. Maybe a maintainer can try triggering a rerun of the build?

[IanCaio]

LGTM!

[Veratil]

I think we have a memory leak (not directly related to this PR's changes). In getName we use malloc to allocate space for both deviceName and endPointName, but then combine them into fullname, leaving both allocations behind without freeing.

Can you add after the sprintf and before the return:

	sprintf(fullName, "%s:%s", deviceName,endPointName);
	if (deviceName != nullptr) { free(deviceName); }
	if (endPointName != nullptr) { free(endPointName); }
	return fullName;

[TheKnarf]

@Veratil the code seems to leak everyplace getName is used in the file...

[Veratil]

@Veratil the code seems to leak everyplace getName is used in the file...

Let's fix them all then while you're there! 😁

[TheKnarf]

I'm not about to do that in this PR. Perhaps file an issue for later?

[Veratil]

I'm not about to do that in this PR. Perhaps file an issue for later?

There are only 4 other locations in that file that use getName. Why not?

[TheKnarf]

@Veratil It's unrelated to the bug I'm trying to fix. It would probably be better to refactor getName to return a std::string or QString. And I was hoping to go to bed.

[Veratil]

Fair enough.