tests: replace tmpnam use
#3497
Conversation
Saviq
force-pushed
the
fix-tmpnam-use
branch
7 times, most recently
from
6c7a94a
to
3f2552f
Compare
| std::string mir::test::create_temp_file() | ||
| { | ||
| char temp_file[] = "/tmp/temp_file_XXXXXX"; | ||
| if (int fd = mkstemp(temp_file) == -1) | ||
| { | ||
| throw std::system_error(errno, std::system_category(), "Failed to create temp file"); | ||
| } | ||
| else | ||
| { | ||
| close(fd); | ||
| } | ||
|
|
||
| return temp_file; | ||
| } |
There was a problem hiding this comment.
So, this suffers basically all the problems that tmpnam has.
But it's only used in test code where the problems of tmpnam aren't really problems, so 🤷♀️
But it would be reasonably easy to replace the current uses of tmpname with mkstemp directly. So even more 🤷♀️
There was a problem hiding this comment.
So, this suffers basically all the problems that
tmpnamhas.
Yeah this is just about silencing the linker warning. But you're right, we can just use mkstemp directly - thanks for the reality check.
There was a problem hiding this comment.
So, this suffers basically all the problems that
tmpnamhas.Yeah this is just about silencing the linker warning. But you're right, we can just use
mkstempdirectly - thanks for the reality check.
I concur
| @@ -43,7 +45,7 @@ struct ExternalClient : miral::TestServer | |||
| miral::X11Support x11_disabled_by_default{}; | |||
| miral::X11Support x11_enabled_by_default{miral::X11Support{}.default_to_enabled()}; | |||
|
|
|||
| std::string const output = tmpnam(nullptr); | |||
| std::string const output = mir::test::create_temp_file(); | |||
There was a problem hiding this comment.
This could be a mir::Fd from mkstmp; the external_client.launch lines would then redirect to the fd using >&$FD.
| auto filename = mir::test::create_temp_file(); | ||
| unlink(filename.c_str()); |
There was a problem hiding this comment.
This could be create_temp_dir plus an arbitrary filename.
| @@ -194,7 +195,7 @@ TEST_F(DesktopFileManager, can_resolve_from_valid_flatpak_info) | |||
| std::stringstream flatpak_info_path; | |||
| flatpak_info_path << "/proc/" << PID << "/root/.flatpak-info"; | |||
| auto const flatpak_info = flatpak_info_path.str(); | |||
| auto tmp_file_name = std::tmpnam(NULL); | |||
| auto tmp_file_name = mir::test::create_temp_file().c_str(); | |||
There was a problem hiding this comment.
This could be mkstmp; nothing really uses the filename here.
| @@ -228,7 +229,7 @@ TEST_F(DesktopFileManager, app_id_will_not_resolve_from_flatpak_info_when_name_i | |||
| std::stringstream flatpak_info_path; | |||
| flatpak_info_path << "/proc/" << PID << "/root/.flatpak-info"; | |||
| auto const flatpak_info = flatpak_info_path.str(); | |||
| auto tmp_file_name = std::tmpnam(NULL); | |||
| auto tmp_file_name = mir::test::create_temp_file().c_str(); | |||
Fixes #2675