lkml.org 
[lkml]   [2008]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/2] cgroup map files: Add cgroup map data type
On Sat, Feb 23, 2008 at 12:04 AM, Andrew Morton
<akpm@linux-foundation.org> 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


\
 
 \ /
  Last update: 2008-02-23 16:25    [W:0.064 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site