| Message ID | 20211220102759.311224-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/qtest/virtio-net-failover: Use g_random_int() instead of g_test_rand_int() | expand |
On 12/20/21 2:27 AM, Thomas Huth wrote: > const gchar *tmpdir = g_get_tmp_dir(); > gchar *tmpfile = g_strdup_printf("%s/failover_test_migrate-%u-%u", > - tmpdir, getpid(), g_test_rand_int()); > + tmpdir, getpid(), g_random_int()); Random numbers plus pid are irrelevant, because you still don't have guaranteed uniqueness -- think stale files in /tmp. Use g_file_open_tmp. r~
On 12/20/21 20:26, Richard Henderson wrote: > On 12/20/21 2:27 AM, Thomas Huth wrote: >> const gchar *tmpdir = g_get_tmp_dir(); >> gchar *tmpfile = g_strdup_printf("%s/failover_test_migrate-%u-%u", >> - tmpdir, getpid(), >> g_test_rand_int()); >> + tmpdir, getpid(), g_random_int()); > > Random numbers plus pid are irrelevant, because you still don't have > guaranteed uniqueness -- think stale files in /tmp. > > Use g_file_open_tmp. Another use in test_socket_unix_abstract(), tests/unit/test-util-sockets.c.
On 20/12/2021 21.02, Philippe Mathieu-Daudé wrote: > On 12/20/21 20:26, Richard Henderson wrote: >> On 12/20/21 2:27 AM, Thomas Huth wrote: >>> const gchar *tmpdir = g_get_tmp_dir(); >>> gchar *tmpfile = g_strdup_printf("%s/failover_test_migrate-%u-%u", >>> - tmpdir, getpid(), >>> g_test_rand_int()); >>> + tmpdir, getpid(), g_random_int()); >> >> Random numbers plus pid are irrelevant, because you still don't have >> guaranteed uniqueness -- think stale files in /tmp. >> >> Use g_file_open_tmp. > > Another use in test_socket_unix_abstract(), > tests/unit/test-util-sockets.c. Using g_file_open_tmp is certainly better ... but the tests are currently written in a way where they require the file name of the temporary file - so switching to g_file_open_tmp() (which only provides a file handle) certainly would need some rewrite here... Thus I'd suggest to go first with this patch to silence the Assert messages, and then to clean this up properly later. Thomas
On 12/21/21 2:32 AM, Thomas Huth wrote: > Using g_file_open_tmp is certainly better ... but the tests are currently written in a way > where they require the file name of the temporary file - so switching to g_file_open_tmp() > (which only provides a file handle) certainly would need some rewrite here... Incorrect. g_file_open_tmp returns the open file handle, but also returns the filename in *name_used. I should think you can close the file handle straight away and use the filename. r~
On 21/12/2021 20.32, Richard Henderson wrote: > On 12/21/21 2:32 AM, Thomas Huth wrote: >> Using g_file_open_tmp is certainly better ... but the tests are currently >> written in a way where they require the file name of the temporary file - >> so switching to g_file_open_tmp() (which only provides a file handle) >> certainly would need some rewrite here... > > Incorrect. g_file_open_tmp returns the open file handle, but also returns > the filename in *name_used. D'oh - not sure how I missed that yesterday :-/ > I should think you can close the file handle straight away and use the > filename. Ok, then let's continue with that approach. Thomas
diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c index 4b2ba8a106..70b94bf18f 100644 --- a/tests/qtest/virtio-net-failover.c +++ b/tests/qtest/virtio-net-failover.c @@ -1308,7 +1308,7 @@ int main(int argc, char **argv) { const gchar *tmpdir = g_get_tmp_dir(); gchar *tmpfile = g_strdup_printf("%s/failover_test_migrate-%u-%u", - tmpdir, getpid(), g_test_rand_int()); + tmpdir, getpid(), g_random_int()); int ret; g_test_init(&argc, &argv, NULL);
g_test_rand_int() must not be called before g_test_init(), otherwise the glib will show a "g_rand_int: assertion 'rand != NULL' failed" message in the log. So we could change the order here, but actually, g_test_rand_int() seems also the wrong choice here, since we're using it to create a temporary file name, which certainly should not depend on the "--seed" CLI option of the g_test suite. Thus let's use the g_random_int() function instead. Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/qtest/virtio-net-failover.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)