On Mon, Nov 18, 2013 at 04:38:03PM -0800, Linus Torvalds wrote: > Hmm.. Al - this looks like a major oversight, but it also looks like > the wrong place to initialize count/from in, just because it doesn't > follow any sane patterns. > > My gut feel is that this needs more cleanup and some sane helper > function that always initializes those fields when allocating a new > buffer. Rather than the "initialize in random places and then miss a > few". > > Afaik, those fields currently get (re-)initialized when: > > - We do the memset() of the whole seq_file structure at seq_open() time. > > - at the top of traverse() > > - count (but not from) gets reinitialized when growing the buffer or > after traverse() fails in seq_read() > > and it really doesn't give me that happy fuzzy feeling of "that all > makes sense". Charley's patch seems to fix a missing initialization, > but I'd *really* like to have it all make more sense, and feel that > we're not missing some *other* initialization. > > Al?
See upthread. The bug is real, but I would rather go for a different fix; it's not worth helper functions, though - we have exactly two places where free m->buf without freeing m itself, and all we need to do is clearing m->count in those two places. No point delaying that to the next call of seq_read() (and no point cleaning m->from at all), as soon as we free m->buf we obviously lose all the data that might've been in it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
