panic on memory allocation failure

[David Disseldorp]

Hi Jeremy,

On Wed, 19 Feb 2014 17:13:41 -0800, Jeremy Allison wrote:

> On Wed, Feb 19, 2014 at 08:20:04PM +0100, Andreas Schneider wrote:
> > The branch, master has been updated  
> 
> Andreas and David,
> 
> >        via  b8844fc clitar.c: check all allocations return value  
> 
> I *HATE* *HATE* *HATE* the idiom used here.
> 
> Calling smb_panic on an allocation fail in the
> new tar code is manifestly the *WRONG* thing to
> do.

I disagree. Graceful handling of memory allocation failures is in most
cases completely untested and adds a huge amount of unnecessary
error-path complexity.

IMO it only makes some sense in library code, where the calling
application may wish to handle an allocation failure in a different
way, but in client and server code a panic is better, or at least
no worse than falling over in an untested and unpredictable way, or
being killed by the OOM slayer.

...

> Remember on common setups this will call a
> panic action script which will leave the
> client hanging on a sleep 9999999 call and
> leave the caller with a hung command and no
> indication of what failed.

Sleep is used for developer builds, but the default smb.conf panic
action is to attempt to dump core and move on.

> Can you fix this please to do proper error
> checking of a NULL return and to bail out
> of the tar command with a correct error indication
> that the command failed ?
> 
> This really needs fixing properly before this code
> can go anywhere near a production branch.

Do you (or others) feel so strongly about this? I'd prefer to keep it as
is.

Cheers, David