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

[Dave Chinner <david@fromorbit.com>]

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