On Mon, Mar 26, 2012 at 18:37, Steve Kargl
s...@troutmask.apl.washington.edu wrote:
On Mon, Mar 26, 2012 at 06:17:08PM +0300, Janne Blomqvist wrote:
currently in libgfortran we have two malloc() wrappers, get_mem and
internal_malloc_size, which abort the program if malloc fails.
internal_malloc_size does
if (size == 0)
size = 1;
and then calls get_mem, which doesn't have such a zero-size check.
This is, however, a bug in get_mem, as malloc(0) is allowed to return
NULL, and in that case get_mem would incorrectly abort the program.
Thus having both these functions is unnecessary. The attached patch
combines these two functions into one. As I couldn't decide between
get_mem and internal_malloc_size and didn't particularly like either
of the names, I named the combined function xmalloc, as is common
for such functions to be called.
When developing the patch, I noticed that the implementations of the
intrinsics IALL, IANY, IPARITY, NORM2, and PARITY weren't being
regenerated from the m4 sources. The reason, it turned out, was that
the dependencies were specified incorrectly in Makefile.am. The patch
also fixes this.
Regtested on x86_64-unknown-linux-gnu. While the patch is large, it's
also mechanical, hence committed as obvious.
The patch looks ok to me, but I do have two questions.
1) xmalloc is used in other parts of gcc. Is there a
possibility of confusing the libgfortran version
with the other(s)? Perhaps, a unique name is in
order, gfc_xmalloc().
I though about that, and ultimately decided against it, as the symbol
name is mangled anyway (__gfortrani_xmalloc IIRC), and so shouldn't
cause any linking problems. Also, I don't think it's worth worrying
about similar named source identifiers in other subdirectories in the
GCC tree; it's easy enough to find the implementation that is used in
libgfortran with (git) grep.
2) Unless I've misread the patch (which is always a
possibility), it seems that you've removed
GFC_CLEAR_MEMORY, which used calloc to zero the
allocated memory. Is there a situation where
libgfortran needs/assumes the memory is zeroed
upon allocation?
GFC_CLEAR_MEMORY was a macro for debugging purposes. Today, with tools
like valgrind available, replacing malloc with calloc is quite a
inefficient debugging tool. And as it's only a one-line change to
replace the malloc call with calloc inside xmalloc, if somebody wants
to do that it's no more effort than uncommenting the GFC_CLEAR_MEMORY
macro definition was before.
And yes, we nowadays have a corresponding xcalloc for those cases
where the caller requires zeroed memory; before xcalloc the caller
used memset to explicitly zero the memory, so GFC_CLEAR_MEMORY was
never needed from a correctness standpoint.
--
Janne Blomqvist
