Rob Jones <rob.jones@xxxxxxxxxxxxxxx> writes: > On 06/08/14 17:02, Al Viro wrote: >> On Tue, Jul 29, 2014 at 06:39:53PM +0100, Rob Jones wrote: >> >>> At the moment these consumers have to obtain the struct seq_file pointer >>> (stored by seq_open() in file->private_data) and then store a pointer to >>> their own data in the private field of the struct seq_file so that it >>> can be accessed by the iterator functions. >>> >>> Although this is not a long piece of code it is unneccessary boilerplate. >> >> How many of those do we actually have? > > A quick grep (I didn't examine them all) showed what looked like at > least 80 instances of the work around. I took a quick look as well and what I saw was that if we were to implement the helpers: seq_open_PDE_DATA, and seq_open_i_private. That would cover essentially all of seq_open that set seq_file->private. So my gut feel is that a seq_open_priv is the wrong helper. In the vast majority of the cases either seq_open is good enough, we want seq_open_private, or seq_file->private is set to PDE_DATA or i_private. I think there may be 5 cases in the kernel that do something different, and those cases probably need a code audit. >>> seq_open() remains in place and its behaviour remains unchanged so no >>> existing code should be broken by this patch. >> >> I have no objections against such helper, but I's rather have it >> implemented via seq_open() (and as a static inline, not an export), >> not the other way round. Oh, and conversion of at least some users would >> be nice to have as well... I have no significant objection but having both seq_open_private and seq_open_priv seems confusing name wise. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
