Re: [Libcg-devel] [PATCH] Better error reporting for lincgroup

[Balbir S. <ba...@li...>]

* Dhaval Giani <dh...@li...> [2008-12-01 20:04:27]:

> On Mon, Dec 01, 2008 at 08:00:38PM +0530, Balbir Singh wrote:
> > * Ankita Garg <an...@in...> [2008-12-01 17:25:52]:
> > 
> > > Hi Balbir,
> > > 
> > > Sure, no problem. Sorry could not clean up the patch after the workout..
> > 
> > How does this look?
> > 
> > 
> > This patch adds human readable error messages to the configuration parser.
> > The error lookup is implemented as a simple table, indexed by the error.
> > 
> > TODO: Save errno at ECGOTHER and pass it down as cgroup_lasterror.
> > 
> > Signed-off-by: Balbir Singh <ba...@li...>
> > Signed-off-by: Ankita Garg <an...@in...>
> > 
> > 
> > Index: trunk/api.c
> > ===================================================================
> > --- trunk.orig/api.c	2008-12-01 15:58:11.000000000 +0530
> > +++ trunk/api.c	2008-12-01 19:37:35.000000000 +0530
> > @@ -40,6 +40,7 @@
> >  #include <ctype.h>
> >  #include <pwd.h>
> >  #include <libgen.h>
> > +#include <assert.h>
> > 
> >  #ifndef PACKAGE_VERSION
> >  #define PACKAGE_VERSION 0.01
> > @@ -70,6 +71,31 @@
> >  /* Lock for the list of rules (rl) */
> >  static pthread_rwlock_t rl_lock = PTHREAD_RWLOCK_INITIALIZER;
> > 
> > +char *cgroup_strerror_codes[] = {
> > +	"Cgroup is not compiled in",
> > +	"Cgroup is not mounted",
> > +	"Cgroup does not exist",
> > +	"Cgroup has not been created",
> > +	"Cgroup one of the needed subsystems is not mounted",
> > +	"Cgroup, request came in from non owner",
> > +	"Cgroup controllers controllers are bound to different mount points",
> > +	"Cgroup, operation not allowed",
> > +	"Cgroup value set exceeds maximum",
> > +	"Cgroup controller already exists",
> > +	"Cgroup value already exists",
> > +	"Cgroup invalid operation",
> > +	"Cgroup, creation of controller failed",
> > +	"Cgroup operation failed",
> > +	"Cgroup not initialized",
> > +	"Cgroup trying to set value for control that does not exist",
> > +	"Cgroup generic error, see errno",
> > +	"Cgroup values are not equal",
> > +	"Cgroup controllers are different",
> > +	"Cgroup parsing failed",
> > +	"Cgroup, rules file does not exist",
> > +	"Cgroup mounting failed",
> > +};
> > +
> 
> It might also be a good idea to add the error code as a comment to the
> right so that we know what is what. Not needed for this iteration, but
> would be a good thing to have.
>

Good suggestion, will do!
 
> >  static int cg_chown_file(FTS *fts, FTSENT *ent, uid_t owner, gid_t group)
> >  {
> >  	int ret = 0;
> > @@ -2077,3 +2103,9 @@
> >  	free(path);
> >  	return ret;
> >  }
> > +
> > +char *cgroup_strerror(int code)
> > +{
> > +	assert((code >= ECGROUPNOTCOMPILED) && (code < ECGSENTINEL));
> 
> This means everytime we add a new error code we break ABI? (Sorry if the
> question is too dumb)
> 

I don't understand the breaking the ABI part? ECGSENTINEL is an
invalid error code. We never use that in the program, we check to see
if code < ECGSENTINEL.


-- 
	Balbir