Re: [PATCH] grep: don't segfault

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jan 18, 2010, 22:40 UTC
Message-ID: <alpine.LFD.2.00.1001181433240.13231@localhost.localdomain>
In-Reply-To: <871vhnhydg.fsf@meyering.net>

On Mon, 18 Jan 2010, Jim Meyering wrote:

Show 14 quoted lines

> 
> 	sz = xsize_t(st.st_size);
> 	i = open(filename, O_RDONLY);
> 	if (i < 0)
> 		goto err_ret;
> 	data = xmalloc(sz + 1);
> 	if (st.st_size != read_in_full(i, data, sz)) {
> 		error("'%s': short read %s", filename, strerror(errno));
> 		close(i);
> 		free(data);
> 		return 0;
> 	}
> 	close(i);
> >>>     data[sz] = 0;    <======  added line

Heh. Fredrik's threaded grep patch also fixed this, although he did the "data[*sz] = 0;" up front.

So an obvious ack on the whole thing (the xmalloc() itself makes it obvious that that thing should be NULL-terminated).

That said, I also suspect that we have a _lot_ of these patterns, and I wonder if we should just add a

	void *read_file(int fd, struct stat *st);

helper function, which reads a file and NULL-terminates it. Doing a

	git grep -5 read_in_full

on the git sources, and it looks like there are several cases that basically do that xmalloc+read_in_full+error handling pattern.

		Linus