Agree with all the suggestions below. These are great ideas. Will make the changes.
Thanks, -Rob Foley On Thu, 7 Nov 2019 at 11:32, Alex Bennée <alex.ben...@linaro.org> wrote: > > > Robert Foley <robert.fo...@linaro.org> writes: > > > One test ensures that the logfile handle is still valid even if > > the logfile is changed during logging. > > The other test validates that the logfile handle remains valid under > > the logfile lock even if the logfile is closed. > > > > Signed-off-by: Robert Foley <robert.fo...@linaro.org> > > --- > > tests/test-logging.c | 74 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 74 insertions(+) > > > > diff --git a/tests/test-logging.c b/tests/test-logging.c > > index a12585f70a..a3190ff92c 100644 > > --- a/tests/test-logging.c > > +++ b/tests/test-logging.c > > @@ -108,6 +108,76 @@ static void test_parse_path(gconstpointer data) > > error_free_or_abort(&err); > > } > > > > +static void test_logfile_write(gconstpointer data) > > +{ > > + QemuLogFile *logfile; > > + gchar const *dir = data; > > + Error *err = NULL; > > + gchar *file_path; > > + gchar *file_path1; > > with g_autofree char *file_path you can avoid the free down bellow. > > > + FILE *orig_fd; > > + > > + file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL); > > + file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL); > > + > > + /* > > + * Test that even if an open file handle is changed, > > + * our handle remains valid due to RCU. > > + */ > > + qemu_set_log_filename(file_path, &err); > > + g_assert(!err); > > + rcu_read_lock(); > > + logfile = atomic_rcu_read(&qemu_logfile); > > + orig_fd = logfile->fd; > > + g_assert(logfile && logfile->fd); > > + fprintf(logfile->fd, "%s 1st write to file\n", __func__); > > + fflush(logfile->fd); > > + > > + /* Change the logfile and ensure that the handle is still valid. */ > > + qemu_set_log_filename(file_path1, &err); > > + g_assert(!err); > > Maybe better would be: > > logfile2 = atomic_rcu_read(&qemu_logfile); > g_assert(logfile->fd == orig_fd); > g_assert(logfile2->fd != logfile->fd); > fprintf(logfile2->fd, "%s 2nd write to file\n", __func__); > fflush(logfile2->fd); > > <snip> > > + g_assert(logfile->fd == orig_fd); > > + fprintf(logfile->fd, "%s 2nd write to file\n", __func__); > > + fflush(logfile->fd); > > + rcu_read_unlock(); > > + > > + g_free(file_path); > > + g_free(file_path1); > > +} > > + > > +static void test_logfile_lock(gconstpointer data) > > +{ > > + FILE *logfile; > > + gchar const *dir = data; > > + Error *err = NULL; > > + gchar *file_path; > > g_autofree > > > + > > + file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL); > > + > > + /* > > + * Test the use of the logfile lock, such > > + * that even if an open file handle is closed, > > + * our handle remains valid for use due to RCU. > > + */ > > + qemu_set_log_filename(file_path, &err); > > + logfile = qemu_log_lock(); > > + g_assert(logfile); > > + fprintf(logfile, "%s 1st write to file\n", __func__); > > + fflush(logfile); > > + > > + /* > > + * Initiate a close file and make sure our handle remains > > + * valid since we still have the logfile lock. > > + */ > > + qemu_log_close(); > > + fprintf(logfile, "%s 2nd write to file\n", __func__); > > + fflush(logfile); > > + qemu_log_unlock(logfile); > > + > > + g_assert(!err); > > + g_free(file_path); > > +} > > + > > /* Remove a directory and all its entries (non-recursive). */ > > static void rmdir_full(gchar const *root) > > { > > @@ -134,6 +204,10 @@ int main(int argc, char **argv) > > > > g_test_add_func("/logging/parse_range", test_parse_range); > > g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path); > > + g_test_add_data_func("/logging/logfile_write_path", > > + tmp_path, test_logfile_write); > > + g_test_add_data_func("/logging/logfile_lock_path", > > + tmp_path, test_logfile_lock); > > > > rc = g_test_run(); > > > -- > Alex Bennée