lkml.org 
[lkml]   [2003]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subjectseq_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]
\
 
 \ /
  Last update: 2005-03-22 13:58    [W:0.048 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site