Re: [PATCH 1/3] mkfs/proto.c: avoid to use NULL pointer

Dave Chinner
2022-06-06T21:46:21Z

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@fromorbit.com