On Sun, Jan 04, 2009 at 10:19:45AM +0530, Sudhir Kumar wrote:
> On Fri, Jan 02, 2009 at 05:55:14PM +0530, Dhaval Giani wrote:
> > At a number of places I noticed a following combination of functions
> > 
> > build_path;
> > strcat;
> > 
> > Realizing that the way build_path is designed is not very conducive
> > to such a scenario, and also requires memory to have been pre-allocated
> > which caused a whole lot of other issues, such as using statically sized
> > arrays which could overflow, not giving us the opportunity to reallocate,
> > it looks like to have this sort of a function is a better option. We can
> > now have dynamically allocated strings which will help us move from static
> > arrays to dynamic arrays.
> > 
> > As of now, only build tested
> > 
> > Changes from v1
> > 1. Used vasprintf as pointed out by Dan Smith. Minimal testing shows the
> > usage is correct, but would require closer review
> > 2. Corrected some other bugs which were pointed out in testing
> > 3. Minimally tested using the test cases Sudhir has submitted.
> > 
> > Signed-off-by: Dhaval Giani <[email protected]>
> > 
> > ---
> >  api.c |  154 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 125 insertions(+), 29 deletions(-)
> > 
> > Index: trunk/api.c
> > ===================================================================
> > --- trunk.orig/api.c        2009-01-02 14:21:37.000000000 +0530
> > +++ trunk/api.c     2009-01-02 14:23:34.000000000 +0530
> > @@ -41,6 +41,7 @@
> >  #include <pwd.h>
> >  #include <libgen.h>
> >  #include <assert.h>
> > +#include <stdarg.h>
> > 
> >  #ifndef PACKAGE_VERSION
> >  #define PACKAGE_VERSION 0.01
> > @@ -687,7 +688,68 @@ static inline pid_t cg_gettid()
> >     return syscall(__NR_gettid);
> >  }
> > 
> > +/*
> > + * Always take the mount table lock before calling in here.
> > + */
> > +static char *cg_build_path_var(char *name, char *type, const char *fmt, 
> > ...)
> > +{
> > +   char *path = NULL, *tmp = NULL, *tmp2 = NULL;
> > +   int i, size, ret;
> > +   bool found = false;
> > +   va_list args;
> > +
> > +   for (i = 0; cg_mount_table[i].name[0] != '\0'; i++) {
> > +           if (strcmp(cg_mount_table[i].name, type) == 0) {
> > +                   found = true;
> > +                   if (name)
> > +                           asprintf(&tmp, "%s/%s", cg_mount_table[i].path,
> > +                                           name);
> > +                   else
> > +                           asprintf(&tmp, "%s", cg_mount_table[i].path);
> > +                   if (!name)
> > +                           return NULL;
> 
> Why dont you test it before? tmp as been allocated and if name is null
> you will miss to free tmp.
> 

Hmm. this is a mistake. I am sure I corrected it, but I will correct it
again in the next version. I need to test the return value of asprintf
and on failure return to NULL. I am not sure how this check got missed.

thanks,
-- 
regards,
Dhaval

------------------------------------------------------------------------------
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to