[Development] QtCore missing check for memory allocation

[Thiago Macieira]

On Friday 27 February 2015 09:20:54 Oswald Buddenhagen wrote:
> the whole point would be *not* using unwrapped malloc and new(nothrow).
> this can be trivially verified for our own code with a grepping bot.

There's an easier solution.

See the reply to Louai.

> > > then explain edd2d9bd0a7f5dbe059aea0902d519b728acc01a.
> > 
> > Who says that's good code? I fixed a warning, I didn't make the code good.
> > 
> > If the compiler starts removing the undefined behaviour, we'll get a test
> > failure and we can easily correct it. We can't do that in a library
> > because we don't know when it will fail.
> 
> clang suggests to do precisely what the patch did. are you saying it is
> making an unsafe suggestion?

I'm suggesting that Clang suggested something that works for Clang today. That 
does not mean it will work:
 * for other compilers
 * for past or future versions of Clang

Undefined behaviour is undefined behaviour. The compiler can implement whatever 
it chooses.

> > The argument is that even if we happened to catch all failures to malloc()
> > in all libraries, there's also all those pesky system calls that may
> > return ENOMEM and we're not checking those. So wrapping malloc() and
> > operator new are not enough: we need to check system calls too. And
> > extend that to all the calls to libraries that do catch malloc() and
> > system call failures and report errors to us.
> 
> the failure to check the return value of system calls needs to be
> addressed irrespective of what we decide about memory management. it's
> simply irresponsible to not handle syscall return values, and can even
> lead to security holes.

That may be.

> > The argument is that chain is as strong as its weakest link. If we don't
> > fix all the holes, the bucket will still leak (pardon my switch of
> > metaphors).
> you can let this be somebody else's worry. our task is to enable a use
> case, not to make it bullet-proof ourselves. the first step is not being
> actively hostile to it.

I am actively against it while it's a huge burden on us for little perceived 
benefit. You have to convince me of the benefit against the cost.

> > And even if I had no technical arguments, my opinion counts. I'm not
> > pulling this out of thin air. I have over a decade of experience, so if I
> > say that it sounds wrong, it probably is. And I'm the maintainer for
> > QtCore, so I'm the one that needs to be convinced, not the other way
> > around.
> 
> you can pull an argument to authority when no unanswered technical
> arguments remain and it's still a draw.

I can pull this when I think this isn't technical, but social. The problem I 
perceive is that the solution you're proposing is a hugely complex task, 
requiring constant maintenance, places a big burden on all our developers, and 
is a leaking bucket. You're going to run into Pareto's Law really quickly too.

And we have better things to do.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center