seq_file API strangeness

[Harald Welte <laforge@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

Hi!

While porting /proc/net/ip_conntrack over to seq_file, I stumbled across
the following problem:

The documentation says:

 *	seq_open() sets @file, associating it with a sequence described
 *	by @op.  @op->start() sets the iterator up and returns the first
 *	element of sequence. @op->stop() shuts it down.  @op->next()
 *	returns the next element of sequence.  @op->show() prints element
 *	into the buffer.  In case of error ->start() and ->next() return
 *	ERR_PTR(error).  In the end of sequence they return %NULL. ->show()
 *	returns 0 in case of success and negative number in case of error.

Now let's say I'm allocating some chunk of memory in ->start(), and
later on an error occurs.  Now I return ERR_PTR(something).  Later on, 
->stop() is called with that ERR_PTR(something) as parameter, and I try
to kfree() the chunk of memory that was allocated.  boom.  It's neither
NULL nor a valid pointer.

Also, I am wondering why the ->stop() function is called at all, when
->start() fails.  Initially, I was grabbing a lock, but only at the end
of ->start(), after all potential errors would already result in
returning ERR_PTR(something).  ->stop() however is then called
unconditionally, resulting in an unconditional unlock of my lock. boom.

Was this by intention?  I think it is unusual to call a  stop() function
even if start() didn't succeed.

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]