Re: [PATCH v5 4/6] selftests/mm/vm_util: robust write_file()

Chunyu Hu
2026-03-24T00:18:52Z

On Mon, Mar 23, 2026 at 03:39:41PM -0700, Andrew Morton wrote:
> On Mon, 23 Mar 2026 23:17:51 +0800 Chunyu Hu <chuhu@redhat.com> wrote:
>
> > Add two more checks for buflen and numwritten. The buflen should be at
> > least one, otherwise the 'buflen - 1' could underflow and cause trouble.
> > The numwritten should be equal to 'buflen - 1'. The test will exit if
> > any of these conditions aren't met.
> >
> > Additionally, add more print information when a write failure occurs or
> > a truncated write happens, providing clearer diagnostics.
> >
> > ...
> >
> > --- a/tools/testing/selftests/mm/vm_util.c
> > +++ b/tools/testing/selftests/mm/vm_util.c
> > @@ -769,6 +769,8 @@ void write_file(const char *path, const char *buf, size_t buflen)
> > {
> > int fd;
> > ssize_t numwritten;
> > + if (buflen < 1)
> > + ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen);
> >
> > fd = open(path, O_WRONLY);
> > if (fd == -1)
> > @@ -777,5 +779,9 @@ void write_file(const char *path, const char *buf, size_t buflen)
> > numwritten = write(fd, buf, buflen - 1);
> > close(fd);
> > if (numwritten < 1)
> > - ksft_exit_fail_msg("Write failed\n");
> > + ksft_exit_fail_msg("%s write(%s) failed: %s\n", path, buf,
> > + strerror(errno));
>
> AI review
> (https://sashiko.dev/#/patchset/20260323151753.2576137-1-chuhu@redhat.com)
> points out that `errno' was overwritten by the close(). Maybe. Or
> maybe a successful close() leaves errno alone, dunno.

I'll address this errno part by adding a save and restore before/after
the close().

>
> Apart from that the comments appear fairly minor. Please lmk if you
> think we should proceed as-is.

For the other two minor questions about buflen == 1 and non-null
buffer, user should know a string buffer need to be '\0' ended.
Those are really minor, we can leave as it is.

AI's comment on patch 5 should be other topic, and minor. Let's
leave as it it.

>
> (I'm really trying to slow things down now - we have a *lot* of
> material and a few weeks of consolidation is needed. But selftests/
> tend to get a pass, for obvious reasons)
>