On Mon, Jun 06, 2022 at 09:29:27AM -0500, Eric Sandeen wrote: > On 6/6/22 7:33 AM, zhanchengbin wrote: > > Change malloc to xmalloc. > > The commit log and cover letter say nothing about this, but apparently > "xmalloc" is often defined locally to abort() on a memory failure, so I > guess you are trying to make (some of?) xfsprogs do that. > > First, this doesn't seem to build.... > > Building mkfs > [CC] proto.o > proto.c: In function ‘setup_proto’: > proto.c:73:15: warning: implicit declaration of function ‘xmalloc’; did you mean ‘malloc’? [-Wimplicit-function-declaration] > 73 | buf = xmalloc(size + 1); > | ^~~~~~~ > | malloc > proto.c:73:13: warning: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] > 73 | buf = xmalloc(size + 1); > | ^ > proto.c: In function ‘newregfile’: > proto.c:306:21: warning: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] > 306 | buf = xmalloc(size); > | ^ > [LD] mkfs.xfs > /usr/bin/ld: proto.o: in function `setup_proto': > /src/git/xfsprogs-dev/mkfs/proto.c:73: undefined reference to `xmalloc' > /usr/bin/ld: proto.o: in function `newregfile': > /src/git/xfsprogs-dev/mkfs/proto.c:306: undefined reference to `xmalloc' > collect2: error: ld returned 1 exit status > gmake[2]: *** [../include/buildrules:65: mkfs.xfs] Error 1 > gmake[1]: *** [include/buildrules:36: mkfs] Error 2 > make: *** [Makefile:92: default] Error 2 > > Second, we have calls to malloc all over the codebase, including around > 100 outside of the internal libraries in the various userspace utilities. > Why change only mkfs/db/repair? xmalloc is local to xfs_db (db/malloc.c), it's not part of libxfs. It's local to xfs_db because it is the tool that always runs out of memory on large filesystems. It's just a wrapper that prints "out of memory" and then exits immediately with no chance to recover. So, essentially, it's no different to immediately SEGV on a NULL pointer and dying. > Third, what problem are you solving? Granted, we should be checking every > malloc call, and it seems that we don't. In general, that's because we can't do anything useful to free memory when we fail memory allocation - we generally exit immediately on memory allocation failure, so checking for failure is almost redundant.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
