Messages in this thread | | | Date | Fri, 14 Nov 2003 20:23:27 +0000 | From | viro@parcelfa ... | Subject | Re: seq_file API strangeness |
| |
On Fri, Nov 14, 2003 at 09:06:43PM +0100, Harald Welte wrote: > Hi! > > While porting /proc/net/ip_conntrack over to seq_file, I stumbled across > the following problem:
> 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.
It is intentional. In 99% of cases it ends up with cleaner methods and in the rest you can trivially check the ->stop() argument.
Note that you *can* have failing ->next() do cleanup if you want to do so. In other words, instances that want such behaviour can get it easily. And in common case you don't have to bother at all.
With "we call ->stop() only if..." you would still have to do the same amount of work for hard cases *AND* get extra PITA for normal ones. IOW, clear loss. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |