On Mon, 29 Nov 2004 13:33:10 PST, Greg KH wrote:
> On Mon, Nov 29, 2004 at 10:46:00AM -0800, Gerrit Huizenga wrote:
> > +++ linux-2.6.10-rc2/include/linux/ckrm_events.h 2004-11-19
> > 20:40:52.517303823 -0800
[... snip ...]
> > +#ifdef CONFIG_CKRM
>
> This ifdef is not needed.
Done - globally.
> > +enum ckrm_event {
>
> Please use a __bitwise marker for your enum so that sparse will check
> for improper usage of it.
Added this to the todo list - we've played with sparse a bit but
I haven't figured out how to do it for enum's correctly yet. But
will do so soon.
> > +#ifdef __KERNEL__
>
> Not needed (see the recent thread on lkml for why).
Fixed globally (I think all of them were unnecessary in all of our
current patches - none of the header files are used by user level
apps today).
> > +typedef void (*ckrm_event_cb) (void *arg);
> > +
> > +struct ckrm_hook_cb {
> > + ckrm_event_cb fct;
> > + struct ckrm_hook_cb *next;
>
> Why not use the kernel list structures here instead?
Good catch. Added to TODO list. This should be trivial but I didn't
get to it this round.
> > +#else /* !CONFIG_CKRM */
>
> Why have 2 different sections in the same header file for this check?
> Please put them all together.
Yeah - this was stupid, fixed.
> > +#ifdef CONFIG_CKRM
> > +void ckrm_cb_newtask(struct task_struct *);
> > +void ckrm_cb_exit(struct task_struct *);
> > +#else
> > +#define ckrm_cb_newtask(x) do { } while (0)
> > +#define ckrm_cb_exit(x) do { } while (0)
>
> Make these static inlines to get the compiler to check the arguments
> properly.
Yep - done.
> > +extern int get_exe_path_name(struct task_struct *, char *, int);
>
> Should this really be here?
Nope. Gone.
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6.10-rc2/kernel/ckrm/ckrm_events.c 2004-11-19
> > 20:40:52.526302397 -0800
> > @@ -0,0 +1,97 @@
> > +/* ckrm_events.c - Class-based Kernel Resource Management (CKRM)
> > + * - event handling routines
> > + *
> > + * Copyright (C) Hubertus Franke, IBM Corp. 2003, 2004
> > + * (C) Chandra Seetharaman, IBM Corp. 2003
> > + *
> > + *
> > + * Provides API for event registration and handling for different
> > + * classtypes.
> > + *
> > + * Latest version, more details at http://ckrm.sf.net
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + */
> > +
> > +/* Changes
> > + *
> > + * 29 Sep 2004
> > + * Separated from ckrm.c
> > + *
> > + */
>
> No changelogs in files please.
Fixed globally.
> > +#define ECC_PRINTK(fmt, args...) \
> > +// printk("%s: " fmt, __FUNCTION__ , ## args)
>
> Looks like it's unused to me :)
>
> Use pr_debug() if you want debugging stuff, don't make up your own...
Fixed globally.
gerrit
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
ckrm-tech mailing list
https://lists.sourceforge.net/lists/listinfo/ckrm-tech
