Re: [PATCH] selftests: arg_parsing: Ensure data is flushed to disk before reading.

[Andrii Nakryiko]

On Tue, Oct 14, 2025 at 5:39 AM Jiri Olsa <olsaj...@gmail.com> wrote:
>
> On Tue, Oct 14, 2025 at 04:03:23PM +0800, Xing Guo wrote:
> > Recently, I noticed a selftest failure in my local environment. The
> > test_parse_test_list_file writes some data to
> > /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read
> > the data back.  However, after writing data to that file, we forget to
> > call fsync() and it's causing testing failure in my laptop.  This patch
> > helps fix it by adding the missing fsync() call.
> >
> > Signed-off-by: Xing Guo <higuox...@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c 
> > b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> > index bb143de68875..4f071943ffb0 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> > @@ -140,6 +140,7 @@ static void test_parse_test_list_file(void)
> >       fprintf(fp, "testA/subtest2\n");
> >       fprintf(fp, "testC_no_eof_newline");
> >       fflush(fp);
> > +     fsync(fd);
>
>
> could we just close the fp stream instead flushing it twice?
>
> maybe something like below, but not sure ferror will work
> after the fclose call
>
> jirka
>
>
> ---
> diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c 
> b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> index bb143de68875..5a4c1bca2a1e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c
> @@ -139,10 +139,10 @@ static void test_parse_test_list_file(void)
>         fprintf(fp, "testA/subtest # subtest duplicate\n");
>         fprintf(fp, "testA/subtest2\n");
>         fprintf(fp, "testC_no_eof_newline");
> -       fflush(fp);
> +       fclose(fp);

we should probably fclose() after ferror().

but the original fix works, though I think we should do fsync(fp)
instead to say within FILE-based APIs.

pw-bot: cr


>
>         if (!ASSERT_OK(ferror(fp), "prepare tmp"))
> -               goto out_fclose;
> +               goto out_remove;
>
>         init_test_filter_set(&set);
>
> @@ -160,8 +160,6 @@ static void test_parse_test_list_file(void)
>
>         free_test_filter_set(&set);
>
> -out_fclose:
> -       fclose(fp);
>  out_remove:
>         remove(tmpfile);
>  }