Re: Coverage for libltdl/slist.c and fallout

[Ralf Wildenhues]

* Gary V. Vaughan wrote on Tue, Dec 01, 2009 at 12:02:42AM CET:
> On 30 Nov 2009, at 16:01, Ralf Wildenhues wrote:
> > * Gary V. Vaughan wrote on Mon, Nov 30, 2009 at 06:29:54PM CET:
> >> On 29 Nov 2009, at 16:27, Ralf Wildenhues wrote:
> >>> - slist_remove should IMVHO return an SList *, because otherwise there
> >>> is no way to avoid a memory leak.  APIs that force memleaks are bad.
> >> 
> > [[...]]
> >> Anyway, if you are proposing that SListCallback functions passed to
> >> slist_remove should always return SList *, then that does seem like a
> >> worthy addition to me.
> > 
> > No.  I don't propose that.  I only propose it for slist_remove, because
> > that's where otherwise, a memleak is inevitable.  That's just what my
> > patch does, by letting slist_remove return an SList *.
> 
> We are in violent agreement!  But rather than patch it the way you propose
> (which just hides the memory leak you describe from the compiler by casting
> the non-SList * valued return of SListCallback away), I suggest we enforce
> it strictly by adding a new SListRemoveCallback that ensures the callback
> function passed to slist_remove returns an SList *, and use that function
> pointer typedef for just slist_remove (in addition to changing the
> prototype of slist_remove to also return that same SList *).

But that forces slist users to write two callback functions with the
same functionality, if they also want to use it in slist_foreach for
example.  And they don't gain anything in return for that.

> >> SList is designed to manage two types of lists:
> >> 
> >>  (i) things that were specifically designed to be chained, in which
> >>      case the first field of the structures to be chained has to be
> >>      'next':
> >> 
> >>      struct notboxed {
> >>         struct notboxed *next;
> >>         <whatever real data needs to be stored in the element>
> >>      };
> >> 
> > [[...]]
> > I don't believe you that it was really designed to do (i).  If it were,
> > then there were some code using it as such, which I would like to ask
> > you to submit as testsuite addition, so we can see whether, and *how*
> > it works at all, and the coverage may keep us from breaking it.  I don't
> > believe that it works.  Just like the sorting functions did not work.
> 
> It really was.  Early versions of slist.c did not have the boxing concept
> yet, and you had to cast your structs (with next field first) into SList *
> to use the API.
> 
> It's possible (though I would be a little surprised) that the casting mode
> has been broken by the relatively recent additions of boxing...

So, there is two choices: remove the API, or add test coverage.  Which
alternative do you prefer?

Thanks,
Ralf