[Development] QtCore missing check for memory allocation

[Thiago Macieira]

On Wednesday 25 February 2015 20:07:58 Oswald Buddenhagen wrote:
> On Wed, Feb 25, 2015 at 08:01:54AM -0800, Thiago Macieira wrote:
> > On Wednesday 25 February 2015 16:48:44 Ulf Hermann wrote:
> > > >> We should thus do Q_CHECK_PTR on every memory allocation in Qt and we
> > > >> should fix Q_CHECK_PTR so that it works under all circumstances.
> > > > 
> > > > I disagree on both accounts.
> > > 
> > > Could you elaborate a bit?
> > 
> > I disagree that we should do Q_CHECK_PTR after every memory allocation and
> > I disagree that Q_CHECK_PTR should do something in all circumstances.
> elaborate, not restate more verbosely.

I already have, on multiple previous occasions, explained why I don't think 
this is a useful use of our brain cycles and of CPU cycles.

> as ulf pointed out, a rather trivial wrapper which ensures deterministic
> behavior is hardly a burden.

And I disagree that it's hardly a burden. I am saying it is overhead.

Let me put it this way: who's going to write the unit tests to ensure we get 
coverage for all those conditionals? Any volunteers?

> > > > That's undefined behaviour. If you write code:
> > > > 	if (!p)
> > > > 	
> > > > 		*(char*)p = 42;		// crash
> > > 
> > > I'm not saying we should access p in this case, but rather some fixed
> > > place
> > > of which we know we cannot access it, to *reliably* raise a segfault.
> > > Maybe
> > > there is even a more elegant way to trigger a segfault than accessing
> > > invalid memory.
> > 
> > The only reliable way of causing a segfault is raise(SIGSEGV). You can't
> > reliably trigger a memory problem because, by the very definition of it,
> > the compiler is allowed to assume it doesn't happen.
> 
> you can assign to a volatile pointer and deref it. the compiler is not
> allowed to optimize that away. we established that much last time we
> discussed this topic.

Sorry, the compiler *is* allowed to remove undefined behaviour because, by the 
very definition of undefined behaviour, ANYTHING can happen, including 
absolutely nothing.

In any case, raise(SIGSEGV) on Unix is fine. We'd just have to find something 
equivalent for Windows. I see no need for trying to figure out an address that 
crashes -- instead, let's use a system call. It's hardly going to be 
performance-critical anyway.

> > > [...] Q_CHECK_PTR should mean "If the pointer is 0 either throw an
> > > exception or abort right away. Don't just continue."
> > 
> > I understand your arguments, but I still disagree we should act.
> 
> well, and i say you are wrong.
> see the problem with this kind of argumentation?

We've both exposed our technical argumentation for our suggestions but arrived 
at no consensus.

What now? Since we're talking about a Qt-wide policy decision, the Chief 
Maintainer should be asked to weigh in.

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