On Sat, Feb 23, 2008 at 12:04 AM, Andrew Morton
<[EMAIL PROTECTED]> wrote:
> > +static int cgroup_map_add(struct cgroup_map_cb *cb, const char *key, u64
> value)
> > +{
> > + struct seq_file *sf = cb->state;
> > + return seq_printf(sf, "%s %llu\n", key, value);
> > +}
>
> We don't know what type the architecture uses to implement u64. This will
> warn on powerpc, sparc64, maybe others.
OK, I'll add an (unsigned long long) cast.
>
>
> > +static int cgroup_seqfile_show(struct seq_file *m, void *arg)
> > +{
> > + struct cgroup_seqfile_state *state = m->private;
> > + struct cftype *cft = state->cft;
> > + if (cft->read_map) {
> > + struct cgroup_map_cb cb = {
> > + .fill = cgroup_map_add,
> > + .state = m,
> > + };
> > + return cft->read_map(state->cgroup, cft, &cb);
> > + } else {
> > + BUG();
>
> That's not really needed. Just call cft->read_map unconditionally. if
> it's zero we'll get a null-pointer deref which will have just the same
> effect as a BUG.
OK. The long-term plan is to have other kinds of files also handled by
this function, so eventually it would look something like:
if (cft->read_map) {
...
} else if (cft->read_something_else) {
...
}
...
} else {
BUG();
}
But I guess I can save that for the future.
> > static int cgroup_file_open(struct inode *inode, struct file *file)
> > {
> > int err;
> > @@ -1499,7 +1539,18 @@ static int cgroup_file_open(struct inode
> > cft = __d_cft(file->f_dentry);
> > if (!cft)
> > return -ENODEV;
> > - if (cft->open)
> > + if (cft->read_map) {
>
> But above a NULL value is illegal. Why are we testing it here?
>
>
The existence of cft->read_map causes us to open a seq_file. Otherwise
we do nothing special and carry on down the normal open path.
Paul
--
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/
