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
