[Dune] false accusations

[Christian Engwer]

On Fri, Dec 04, 2009 at 05:49:53PM +0100, Martin Nolte wrote:
> Dear Christian,
> 
> I was indeed assuming that you should at least have tested with gcc-4.3. So
> simply return to your version, and run the tests with gcc-4.3.4 (without
> optimization). It will compile but, e.g., YaspGrid will produce a segmentation
> fault.
> 
> For us, the problem occurred in ALUGrid, though, and it took a lot of time to
> figure out where the problem originated from. Hadn't Robert mentioned that
> something in the EntityPointer changed, we would have looked in ALUGrid for
> the bug. Since this is a very central place, we were of the opinion that such
> changes need heavy testing.
> 
> So, in case you really did the required testing, I apologize for the harshness
> of the commit message. There were simply too many commits in the last weeks
> that simply couldn't ever have compiled.
> 
> Maybe you can explain to me why it might work at all: You are calling the copy
> constructor of the entity pointer onto itself, aren't you? Is it safe to
> assume that this constructor will not change anything in memory? As far as I
> know, the ALUGrid entity pointer holds a pointer to an entity. The entity
> pointer's copy constructor then allocates a copy of the entity. Hence, I
> assume this technique might cause a memory leak. Can we further assume that it
> will be optimized out (otherwise we could really copy the entity pointer and
> the cast is not needed)?

As stated in the commit message this isn't the final version, so
efficiency was only my second concern. I'm using a placement new and
all I do is that I "copy" the data onto itself. As one data is read
after the other it shouldn't pose any problems, because the data still
contains the valid version, is then read and the written again. In
case of a pointer and a deep copy I'm afraid you are right, that is a
case I didn't consider in the first place.

Carsten and me are currently discussing (and testing) different
options in order to get rid of the aliasing problem.

We'll come up with a proposal on monday.

Christian

> 
> Martin
> 
> Christian Engwer wrote:
> > From the logs:
> > 
> > ------------------------------------------------------------------------
> > r6036 | mnolte | 2009-12-04 16:47:37 +0100 (Fr, 04. Dez 2009) | 4
> > Zeilen
> > 
> > !!!before committing basic changes run make check!!!
> > 
> > This should simply be natural. It took Andreas, Robert and me 30 minutes to find this bug.
> > 
> > Dear Martin,
> > 
> > first of all, I did run the tests and they worked on my machine.
> > 
> > second, if you revert a patch, you should at least state why it was
> > wrong. So, what were the problems you encountered? The version you
> > submitted is also wrong and gives bogus results if you allow strict
> > aliasing.
> > 
> > Perhaps you aren't the only one thinking in this world, other people
> > do this as well and I submitted this particular version _on purpose_.
> > 
> > Christian
> > 
> > _______________________________________________
> > Dune mailing list
> > Dune at dune-project.org
> > http://lists.dune-project.org/mailman/listinfo/dune
>