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

[Chunyu Hu]

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@xxxxxxxxxx> 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@xxxxxxxxxx)
> 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)
>