Messages in this thread | | | Date | Fri, 14 Nov 2003 21:06:43 +0100 | From | Harald Welte <> | Subject | seq_file API strangeness |
| |
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 [unhandled content-type:application/pgp-signature] | |