Driver porting: The seq_file interface

Posted Nov 14, 2003 13:13 UTC (Fri) by laf0rge (subscriber, #6469)
Parent article: Driver porting: The seq_file interface

After using this article as an example to port the /proc/net/ip_conntrack interface over to seq_file, and about 5 hours of crashing/rebooting/debugging, I have to admit that there are some shortcomings in it.

Some hints for other people, so they don't fall into the same pits as I did:

1) If you allocate something in ct_seq_start(), the place to free it is _NOT_ in ct_seq_stop(). This is because ct_seq_stop() is even called if ct_seq_start() returns an error (Like ERR_PTR(-ENOMEM)). You would then end up calling kfree(ERR_PTR(-ENOMEM)) which your mm subsystem doesn't really like. I am now kfree()ing in ct_seq_next(), just before it returns with NULL at the end of the table.

2) If you take a lock in ct_seq_start(), do it unconditionally as the first thing. Even if ct_seq_start() fails, ct_seq_stop() is called. In ct_seq_stop() you have no idea of knowing if ct_seq_start() failed or not - so you will unconditionally unlock.

Driver porting: The seq_file interface

Posted Nov 15, 2003 1:53 UTC (Sat) by giraffedata (guest, #1954) [Link]

I am now kfree()ing in ct_seq_next(), just before it returns with NULL at the end of the table

Seems like that would be a problem if the user chooses not to read all the way to EOF.

This just sounds like a basic bug in the seq_file interface. If ct_seq_start() fails, it should be ct_seq_start's responsibility to not change any state, and thus ct_seq_stop doesn't need to be, and should not be, called. After all, does a POSIX program call close(-1) when open() fails?