Experimental Physics and
Industrial Control System

Benjamin Franksen <[email protected]> · Tue, 25 Jun 2013 03:19:20 +0200

Hi Andrew and Michael

Am Montag, 24. Juni 2013, 19:16:10 schrieb Andrew Johnson:
> On 2013-06-24 Michael Abbott wrote:
> > On Mon, 24 Jun 2013, Andrew Johnson wrote:
> > > This is a C API, C doesn't have closures
> >
> > Of course C has closures, they just require a little cooperation from the
> > called function part.  A closure in C is a function taking at least one
> > void* parameter together with a void* value to pass to it.
>
> The void* parameter is what we've always regarded as a context pointer for
> the function.  The storage for that data has to be managed by the
> programmer though, so while it performs the same job as a closure it isn't
> a real one.
>
> http://en.wikipedia.org/wiki/Closure_(computer_science) says:
>   "The idiom is similar to closures in functionality, but not in syntax."
>
> > In truth a C++ closure is exactly the same thing, only in disguise.
>
> C++11 has closures which it calls lambda-expressions; they look very
> useful, but do move the language even more in the direction of line-noise
> IMHO.

It does, but this is all completely beside the point.

Whether we call it closure or function-with-a-context or whatsit, the simple
fact is that the void* parameter is something that *belongs to the function*.
Why? Because we must assume that the function casts the void* to something
useful and then accesses the memory behind it. This means such a function
should never be called when there is a good chance that this memory might be
invalid.

But this is exactly what can easily happen with the current API (and parts of
your proposed API, namely the version that is equivalent to the existing one).

This is the scenario:

Some module uses errlogAddListener with a public (globally visible) function
and private data (iocLog.c does that, for instance). Another module uses the
same function with different private data. The second one removes its listener
early. Then the first instance will be removed, instead of the second one, as
might have been (naively) expected from the POV of the developer of the second
module. So subsequent calls to errlogPrintf or its friends will cause the
second listener to be called from the errlog task.

Depending on what the function does with the private data (which might well
become freed after the call to errlogRemoveListener) this could lead to
crashing the IOC, or (even worse) to some completely unrelated memory being
subtly altered. This is, give or take, the worst imaginable case of a failure
because it is extremely hard to debug or even reproduce if you cannot use
things like valgrind (i.e. on a hard IOC, which is exactly where you *need*
the errlog facility).

All this is not just idle fantasy, it is has happened to me, after I tested
and published three versions of a support module that depends on the current
API. It did not show up in any tests because I did not imagine my listener
function to be called /after/ I explicitly removed it from the errlog system.

You may argue that this is a user error. It is. But making such an error is
made a bit too easy by the existing API, which is why I think it is broken and
should be deprecated. Any code using it (including iocLog) potentially
contains a ticking time bomb and is well advised to be reviewed w.r.t. this
potential weakness.

BTW, forget about the matcher function argument in my last proposal, all the
added flexibility is not needed, so it makes no sense to discuss how to design
an API to support it. I challenge you to come up with a real world example
that needs anything besides my originally proposed

        void errlogRemoveListener(listener *, void *);

which should remove all listeners with matching function pointer and private
data pointer.

Cheers
--
Ben Franksen
()  ascii ribbon campaign - against html e-mail
/\  www.asciiribbon.org   - against proprietary attachm€nts

________________________________

Helmholtz-Zentrum Berlin für Materialien und Energie GmbH

Mitglied der Hermann von Helmholtz-Gemeinschaft Deutscher Forschungszentren e.V.

Aufsichtsrat: Vorsitzender Prof. Dr. Dr. h.c. mult. Joachim Treusch, stv. Vorsitzende Dr. Beatrix Vierkorn-Rudolph
Geschäftsführung: Prof. Dr. Anke Rita Kaysser-Pyzalla, Thomas Frederking

Sitz Berlin, AG Charlottenburg, 89 HRB 5583

Postadresse:
Hahn-Meitner-Platz 1
D-14109 Berlin

http://www.helmholtz-berlin.de

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System