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(-)
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);
--
2.27.0
On 12/20/21 11:27, Thomas Huth wrote: > 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(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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
On 20/12/2021 11:27, Thomas Huth wrote:
> 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(-)
>
> 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);
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
© 2016 - 2024 Red Hat, Inc.