[lxc-devel] [PATCH] fix memory leaks in cgroup functions
Dwight Engen
dwight.engen at oracle.com
Sat May 25 02:32:55 UTC 2013
On Fri, 24 May 2013 19:17:14 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > There were several memory leaks in the cgroup functions, notably
> > > in the success cases.
> > >
> > > The cgpath test program was refactored and additional tests added
> > > to it. It was used in various modes under valgrind to test that
> > > the leaks were fixed.
> > >
> > > Simplify lxc_cgroup_path_get() and cgroup_path_get by having them
> > > return a char * instead of an int and an output char * argument.
> > > The only return values ever used were -1 and 0, which are now
> > > handled with NULL and non-NULL returns respectively.
> > >
> > > Use consistent variable names of cgabspath when refering to an
> > > absolute path to a cgroup subsystem or file, and cgrelpath when
> > > refering to a container "group/name" within the cgroup heirarchy.
> >
> > Excellent.
> >
> > > Remove unused subsystem argument to lxc_cmd_get_cgroup_path().
> > >
> > > Remove unused #define MAXPRIOLEN
> > >
> > > Make template arg to lxcapi_create() const
> > >
> > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> >
> > This is great, thanks Dwight.
> >
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> >
> > Except for one thing below,
> >
> > > ---
> > > src/lxc/cgroup.c | 270 ++++++++++++++++++-----------------
> > > src/lxc/cgroup.h | 6 +-
> > > src/lxc/commands.c | 4 +-
> > > src/lxc/commands.h | 3 +-
> > > src/lxc/freezer.c | 36 ++---
> > > src/lxc/lxccontainer.c | 6 +-
> > > src/lxc/lxccontainer.h | 4 +-
> > > src/lxc/state.c | 38 ++---
> > > src/tests/cgpath.c | 372
> > > +++++++++++++++++++++++++++++++++++++------------ 9 files
> > > changed, 463 insertions(+), 276 deletions(-)
> > >
> > > diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> > > index bb1268b..f04d59a 100644
> > > --- a/src/lxc/cgroup.c
> > > +++ b/src/lxc/cgroup.c
> > > @@ -147,102 +147,113 @@ out:
> > > }
> > >
> > > /*
> > > - * cgroup_path_get: Calculate the full path for a particular
> > > subsystem, plus
> > > - * a passed-in (to be appended) relative cgpath for a container.
> > > - * @path: a char** into which a pointer to the answer is copied
> > > - * @subsystem: subsystem of interest (i.e. freezer).
> > > - * @cgpath: a container's (relative) cgroup path, i.e. "/lxc/c1".
> > > + * cgroup_path_get: Get the absolute path to a particular
> > > subsystem,
> > > + * plus a passed-in (to be appended) relative cgpath for a
> > > container. *
> > > - * Returns 0 on success, -1 on error.
> > > + * @subsystem : subsystem of interest (e.g. "freezer")
> > > + * @cgrelpath : a container's relative cgroup path (e.g.
> > > "lxc/c1")
> > > + *
> > > + * Returns absolute path on success, NULL on error. The caller
> > > must free()
> > > + * the returned path.
> > > *
> > > + * Note that @subsystem may be the name of an item (e.g.
> > > "freezer.state")
> > > + * in which case the subsystem will be determined by taking the
> > > string up
> > > + * to the first '.'
> > > */
> > > -extern int cgroup_path_get(char **path, const char *subsystem,
> > > const char *cgpath) +char *cgroup_path_get(const char *subsystem,
> > > const char *cgrelpath) {
> > > int rc;
> > >
> > > char *buf = NULL;
> > > - char *retbuf = NULL;
> > > + char *cgabspath = NULL;
> > >
> > > buf = malloc(MAXPATHLEN * sizeof(char));
> > > if (!buf) {
> > > ERROR("malloc failed");
> > > - goto fail;
> > > + goto out1;
> > > }
> > >
> > > - retbuf = malloc(MAXPATHLEN * sizeof(char));
> > > - if (!retbuf) {
> > > + cgabspath = malloc(MAXPATHLEN * sizeof(char));
> > > + if (!cgabspath) {
> > > ERROR("malloc failed");
> > > - goto fail;
> > > + goto out2;
> > > }
> > >
> > > /* lxc_cgroup_set passes a state object for the
> > > subsystem,
> > > * so trim it to just the subsystem part */
> > > if (subsystem) {
> > > - rc = snprintf(retbuf, MAXPATHLEN, "%s",
> > > subsystem);
> > > + rc = snprintf(cgabspath, MAXPATHLEN, "%s",
> > > subsystem); if (rc < 0 || rc >= MAXPATHLEN) {
> > > ERROR("subsystem name too long");
> > > - goto fail;
> > > + goto err3;
> > > }
> > > - char *s = index(retbuf, '.');
> > > + char *s = index(cgabspath, '.');
> > > if (s)
> > > *s = '\0';
> > > - DEBUG("%s: called for subsys %s name %s\n",
> > > __func__, retbuf, cgpath);
> > > + DEBUG("%s: called for subsys %s name %s\n",
> > > __func__,
> > > + subsystem, cgrelpath);
> > > }
> > > - if (get_cgroup_mount(subsystem ? retbuf : NULL, buf)) {
> > > + if (get_cgroup_mount(subsystem ? cgabspath : NULL, buf))
> > > { ERROR("cgroup is not mounted");
> > > - goto fail;
> > > + goto err3;
> > > }
> > >
> > > - rc = snprintf(retbuf, MAXPATHLEN, "%s/%s", buf, cgpath);
> > > + rc = snprintf(cgabspath, MAXPATHLEN, "%s/%s", buf,
> > > cgrelpath); if (rc < 0 || rc >= MAXPATHLEN) {
> > > ERROR("name too long");
> > > - goto fail;
> > > + goto err3;
> > > }
> > >
> > > - DEBUG("%s: returning %s for subsystem %s", __func__,
> > > retbuf, subsystem);
> > > + DEBUG("%s: returning %s for subsystem %s relpath %s",
> > > __func__,
> > > + cgabspath, subsystem, cgrelpath);
> > > + goto out2;
> > >
> > > - if(buf)
> > > - free(buf);
> > > -
> > > - *path = retbuf;
> > > - return 0;
> > > -fail:
> > > - if (buf)
> > > - free(buf);
> > > - if (retbuf)
> > > - free(retbuf);
> > > - return -1;
> > > +err3:
> > > + free(cgabspath);
> > > + cgabspath = NULL;
> > > +out2:
> > > + free(buf);
> > > +out1:
> > > + return cgabspath;
> > > }
> > >
> > > /*
> > > - * lxc_cgroup_path_get: determine full pathname for a cgroup
> > > - * file for a specific container.
> > > - * @path: char ** used to return the answer.
> > > - * @subsystem: cgroup subsystem (i.e. "freezer") for which to
> > > - * return an answer. If NULL, then the first cgroup entry in
> > > - * mtab will be used.
> > > + * lxc_cgroup_path_get: Get the absolute pathname for a cgroup
> > > + * file for a running container.
> > > + *
> > > + * @subsystem : subsystem of interest (e.g. "freezer"). If NULL,
> > > then
> > > + * the first cgroup entry in mtab will be used.
> > > + * @name : name of container to connect to
> > > + * @lxcpath : the lxcpath in which the container is running
> > > *
> > > * This is the exported function, which determines cgpath from
> > > the
> > > - * monitor running in lxcpath.
> > > + * lxc-start of the @name container running in @lxcpath.
> > > *
> > > - * Returns 0 on success, < 0 on error.
> > > + * Returns path on success, NULL on error. The caller must free()
> > > + * the returned path.
> > > */
> > > -int lxc_cgroup_path_get(char **path, const char *subsystem,
> > > const char *name, const char *lxcpath) +char
> > > *lxc_cgroup_path_get(const char *subsystem, const char *name,
> > > + const char *lxcpath)
> > > {
> > > - int ret;
> > > - char *cgpath;
> > > + char *cgabspath;
> > > + char *cgrelpath;
> > >
> > > - cgpath = lxc_cmd_get_cgroup_path(subsystem, name,
> > > lxcpath);
> > > - if (!cgpath)
> > > - return -1;
> > > + cgrelpath = lxc_cmd_get_cgroup_path(name, lxcpath);
> > > + if (!cgrelpath)
> > > + return NULL;
> > >
> > > - ret = cgroup_path_get(path, subsystem, cgpath);
> > > - free(cgpath);
> > > - return ret;
> > > + cgabspath = cgroup_path_get(subsystem, cgrelpath);
> > > + free(cgrelpath);
> > > + return cgabspath;
> > > }
> > >
> > > /*
> > > - * small helper which simply write a value into a (cgroup) file
> > > + * do_cgroup_set: Write a value into a cgroup file
> > > + *
> > > + * @path : absolute path to cgroup file
> > > + * @value : value to write into file
> > > + *
> > > + * Returns 0 on success, < 0 on error.
> > > */
> > > static int do_cgroup_set(const char *path, const char *value)
> > > {
> > > @@ -267,87 +278,86 @@ static int do_cgroup_set(const char *path,
> > > const char *value) }
> > >
> > > /*
> > > - * small helper to write a value into a file in a particular
> > > directory.
> > > - * @cgpath: the directory in which to find the file
> > > - * @filename: the file (under cgpath) to which to write
> > > - * @value: what to write
> > > + * lxc_cgroup_set_bypath: Write a value into a cgroup file
> > > + *
> > > + * @cgrelpath : a container's relative cgroup path (e.g.
> > > "lxc/c1")
> > > + * @filename : the cgroup file to write (e.g. "freezer.state")
> > > + * @value : value to write into file
> > > *
> > > * Returns 0 on success, < 0 on error.
> > > */
> > > -int lxc_cgroup_set_bypath(const char *cgpath, const char
> > > *filename, const char *value) +int lxc_cgroup_set_bypath(const
> > > char *cgrelpath, const char *filename, const char *value) {
> > > int ret;
> > > - char *dirpath = NULL;
> > > + char *cgabspath;
> > > char path[MAXPATHLEN];
> > >
> > > - ret = cgroup_path_get(&dirpath, filename, cgpath);
> > > - if (ret)
> > > - goto fail;
> > > + cgabspath = cgroup_path_get(filename, cgrelpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > - ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath,
> > > filename);
> > > + ret = snprintf(path, MAXPATHLEN, "%s/%s", cgabspath,
> > > filename); if (ret < 0 || ret >= MAXPATHLEN) {
> > > ERROR("pathname too long");
> > > - goto fail;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > - return do_cgroup_set(path, value);
> > > + ret = do_cgroup_set(path, value);
> > >
> > > -fail:
> > > - if(dirpath)
> > > - free(dirpath);
> > > - return -1;
> > > +out:
> > > + free(cgabspath);
> > > + return ret;
> > > }
> > >
> > > /*
> > > - * set a cgroup value for a container
> > > + * lxc_cgroup_set: Write a value into a cgroup file
> > > *
> > > - * @name: name of the container
> > > - * @filename: the cgroup file (i.e. freezer.state) whose value
> > > to change
> > > - * @value: the value to write to the file
> > > - * @lxcpath: the lxcpath under which the container is running.
> > > + * @name : name of container to connect to
> > > + * @filename : the cgroup file to write (e.g. "freezer.state")
> > > + * @value : value to write into file
> > > + * @lxcpath : the lxcpath in which the container is running
> > > *
> > > * Returns 0 on success, < 0 on error.
> > > */
> > > -
> > > int lxc_cgroup_set(const char *name, const char *filename, const
> > > char *value, const char *lxcpath)
> > > {
> > > int ret;
> > > - char *dirpath = NULL;
> > > + char *cgabspath;
> > > char path[MAXPATHLEN];
> > >
> > > - ret = lxc_cgroup_path_get(&dirpath, filename, name,
> > > lxcpath);
> > > - if (ret)
> > > - goto fail;
> > > + cgabspath = lxc_cgroup_path_get(filename, name, lxcpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > - ret = snprintf(path, MAXPATHLEN, "%s/%s", dirpath,
> > > filename);
> > > + ret = snprintf(path, MAXPATHLEN, "%s/%s", cgabspath,
> > > filename); if (ret < 0 || ret >= MAXPATHLEN) {
> > > ERROR("pathname too long");
> > > - goto fail;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > - return do_cgroup_set(path, value);
> > > + ret = do_cgroup_set(path, value);
> > >
> > > -fail:
> > > - if(dirpath)
> > > - free(dirpath);
> > > - return -1;
> > > +out:
> > > + free(cgabspath);
> > > + return ret;
> > > }
> > >
> > > /*
> > > - * Get value of a cgroup setting for a container.
> > > + * lxc_cgroup_get: Read value from a cgroup file
> > > *
> > > - * @name: name of the container
> > > - * @filename: the cgroup file to read (i.e. 'freezer.state')
> > > - * @value: a preallocated char* into which to copy the answer
> > > - * @len: the length of pre-allocated @value
> > > - * @lxcpath: the lxcpath in which the container is running (i.e.
> > > - * /var/lib/lxc)
> > > + * @name : name of container to connect to
> > > + * @filename : the cgroup file to read (e.g. "freezer.state")
> > > + * @value : a pre-allocated buffer to copy the answer into
> > > + * @len : the length of pre-allocated @value
> > > + * @lxcpath : the lxcpath in which the container is running
> > > *
> > > - * Returns < 0 on error, or the number of bytes read.
> > > + * Returns the number of bytes read on success, < 0 on error
> > > *
> > > - * If you pass in NULL value or 0 len, then you are asking for
> > > the size of the
> > > - * file.
> > > + * If you pass in NULL value or 0 len, the return value will be
> > > the size of
> > > + * the file, and @value will not contain the contents.
> > > *
> > > * Note that we can't get the file size quickly through stat or
> > > lseek.
> > > * Therefore if you pass in len > 0 but less than the file size,
> > > your only @@ -357,25 +367,26 @@ fail:
> > > int lxc_cgroup_get(const char *name, const char *filename, char
> > > *value, size_t len, const char *lxcpath)
> > > {
> > > - int fd, ret = -1;
> > > - char *dirpath = NULL;
> > > + int fd, ret;
> > > + char *cgabspath;
> > > char path[MAXPATHLEN];
> > > - int rc;
> > >
> > > - ret = lxc_cgroup_path_get(&dirpath, filename, name,
> > > lxcpath);
> > > - if (ret)
> > > - goto fail;
> > > + cgabspath = lxc_cgroup_path_get(filename, name, lxcpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > - rc = snprintf(path, MAXPATHLEN, "%s/%s", dirpath,
> > > filename);
> > > - if (rc < 0 || rc >= MAXPATHLEN) {
> > > + ret = snprintf(path, MAXPATHLEN, "%s/%s", cgabspath,
> > > filename);
> > > + if (ret < 0 || ret >= MAXPATHLEN) {
> > > ERROR("pathname too long");
> > > - goto fail;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > fd = open(path, O_RDONLY);
> > > if (fd < 0) {
> > > ERROR("open %s : %s", path, strerror(errno));
> > > - goto fail;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > if (!len || !value) {
> > > @@ -394,47 +405,45 @@ int lxc_cgroup_get(const char *name, const
> > > char *filename, char *value, ERROR("read %s : %s", path,
> > > strerror(errno));
> > > close(fd);
> > > +out:
> > > + free(cgabspath);
> > > return ret;
> > > -fail:
> > > - if(dirpath)
> > > - free(dirpath);
> > > - return -1;
> > > }
> > >
> > > -int lxc_cgroup_nrtasks(const char *cgpath)
> > > +int lxc_cgroup_nrtasks(const char *cgrelpath)
> > > {
> > > - char *dirpath = NULL;
> > > + char *cgabspath = NULL;
> > > char path[MAXPATHLEN];
> > > - int pid, ret, count = 0;
> > > + int pid, ret;
> > > FILE *file;
> > > - int rc;
> > >
> > > - ret = cgroup_path_get(&dirpath, NULL, cgpath);
> > > - if (ret)
> > > - goto fail;
> > > + cgabspath = cgroup_path_get(NULL, cgrelpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > - rc = snprintf(path, MAXPATHLEN, "%s/tasks", dirpath);
> > > - if (rc < 0 || rc >= MAXPATHLEN) {
> > > + ret = snprintf(path, MAXPATHLEN, "%s/tasks", cgabspath);
> > > + if (ret < 0 || ret >= MAXPATHLEN) {
> > > ERROR("pathname too long");
> > > - goto fail;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > file = fopen(path, "r");
> > > if (!file) {
> > > SYSERROR("fopen '%s' failed", path);
> > > - return -1;
> > > + ret = -1;
> > > + goto out;
> > > }
> > >
> > > + ret = 0;
> > > while (fscanf(file, "%d", &pid) != EOF)
> > > - count++;
> > > + ret++;
> > >
> > > fclose(file);
> > >
> > > - return count;
> > > -fail:
> > > - if(dirpath)
> > > - free(dirpath);
> > > - return -1;
> > > +out:
> > > + free(cgabspath);
> > > + return ret;
> > > }
> > >
> > > /*
> > > @@ -654,12 +663,12 @@ char *lxc_cgroup_path_create(const char
> > > *lxcgroup, const char *name)
> > > char buf[LARGE_MAXPATHLEN] = {0};
> > >
> > > - if (create_lxcgroups(lxcgroup) < 0)
> > > - return NULL;
> > > -
> > > if (!allcgroups)
> > > return NULL;
> > >
> > > + if (create_lxcgroups(lxcgroup) < 0)
> > > + goto err1;
> > > +
> > > again:
> > > if (visited) {
> > > /* we're checking for a new name, so start over
> > > with all cgroup @@ -670,9 +679,7 @@ again:
> > > file = setmntent(MTAB, "r");
> > > if (!file) {
> > > SYSERROR("failed to open %s", MTAB);
> > > - if (allcgroups)
> > > - free(allcgroups);
> > > - return NULL;
> > > + goto err1;
> > > }
> > >
> > > if (i)
> > > @@ -730,6 +737,7 @@ next:
> > >
> > > fail:
> > > endmntent(file);
> > > +err1:
> > > free(allcgroups);
> > > if (visited)
> > > free(visited);
> > > @@ -880,7 +888,7 @@ int lxc_cgroup_attach(pid_t pid, const char
> > > *name, const char *lxcpath) int ret;
> > > char *dirpath;
> > >
> > > - dirpath = lxc_cmd_get_cgroup_path(NULL, name, lxcpath);
> > > + dirpath = lxc_cmd_get_cgroup_path(name, lxcpath);
> > > if (!dirpath) {
> > > ERROR("Error getting cgroup for container %s:
> > > %s", lxcpath, name); return -1;
> > > diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> > > index 971311e..747ff5c 100644
> > > --- a/src/lxc/cgroup.h
> > > +++ b/src/lxc/cgroup.h
> > > @@ -23,15 +23,13 @@
> > > #ifndef _cgroup_h
> > > #define _cgroup_h
> > >
> > > -#define MAXPRIOLEN 24
> > > -
> > > struct lxc_handler;
> > > extern int lxc_cgroup_destroy(const char *cgpath);
> > > -extern int lxc_cgroup_path_get(char **path, const char
> > > *subsystem, const char *name, +extern char
> > > *lxc_cgroup_path_get(const char *subsystem, const char *name,
> > > const char *lxcpath); extern int lxc_cgroup_nrtasks(const char
> > > *cgpath); extern char *lxc_cgroup_path_create(const char
> > > *lxcgroup, const char *name); extern int lxc_cgroup_enter(const
> > > char *cgpath, pid_t pid); extern int lxc_cgroup_attach(pid_t pid,
> > > const char *name, const char *lxcpath); -extern int
> > > cgroup_path_get(char **path, const char *subsystem, const char
> > > *cgpath); +extern char *cgroup_path_get(const char *subsystem,
> > > const char *cgpath); #endif diff --git a/src/lxc/commands.c
> > > b/src/lxc/commands.c index 3f21488..169914e 100644
> > > --- a/src/lxc/commands.c
> > > +++ b/src/lxc/commands.c
> > > @@ -335,15 +335,13 @@ static int
> > > lxc_cmd_get_clone_flags_callback(int fd, struct lxc_cmd_req *req,
> > > * particular subsystem. This is the cgroup path relative to the
> > > root
> > > * of the cgroup filesystem.
> > > *
> > > - * @subsystem : the cgroup subsystem of interest (i.e. freezer)
> > > * @name : name of container to connect to
> > > * @lxcpath : the lxcpath in which the container is running
> > > *
> > > * Returns the path on success, NULL on failure. The caller must
> > > free() the
> > > * returned path.
> > > */
> > > -char *lxc_cmd_get_cgroup_path(const char *subsystem, const char
> > > *name,
> > > - const char *lxcpath)
> > > +char *lxc_cmd_get_cgroup_path(const char *name, const char
> > > *lxcpath) {
> > > int ret, stopped = 0;
> > > struct lxc_cmd_rr cmd = {
> > > diff --git a/src/lxc/commands.h b/src/lxc/commands.h
> > > index b5b4788..08bde9c 100644
> > > --- a/src/lxc/commands.h
> > > +++ b/src/lxc/commands.h
> > > @@ -67,8 +67,7 @@ struct lxc_cmd_console_rsp_data {
> > >
> > > extern int lxc_cmd_console(const char *name, int *ttynum, int
> > > *fd, const char *lxcpath);
> > > -extern char *lxc_cmd_get_cgroup_path(const char *subsystem,
> > > - const char *name, const
> > > char *lxcpath); +extern char *lxc_cmd_get_cgroup_path(const char
> > > *name, const char *lxcpath); extern int
> > > lxc_cmd_get_clone_flags(const char *name, const char *lxcpath);
> > > extern char *lxc_cmd_get_config_item(const char *name, const char
> > > *item, const char *lxcpath); extern pid_t
> > > lxc_cmd_get_init_pid(const char *name, const char *lxcpath); diff
> > > --git a/src/lxc/freezer.c b/src/lxc/freezer.c index
> > > 35bf3a7..37a07fd 100644 --- a/src/lxc/freezer.c +++
> > > b/src/lxc/freezer.c @@ -120,19 +120,16 @@ out:
> > >
> > > static int freeze_unfreeze(const char *name, int freeze, const
> > > char *lxcpath) {
> > > - char *nsgroup = NULL;
> > > + char *cgabspath;
> > > int ret;
> > > -
> > > - ret = lxc_cgroup_path_get(&nsgroup, "freezer", name,
> > > lxcpath);
> > > - if (ret)
> > > - goto fail;
> > >
> > > - return do_unfreeze(nsgroup, freeze, name, lxcpath);
> > > + cgabspath = lxc_cgroup_path_get("freezer", name,
> > > lxcpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > -fail:
> > > - if (nsgroup)
> > > - free(nsgroup);
> > > - return -1;
> > > + ret = do_unfreeze(cgabspath, freeze, name, lxcpath);
> > > + free(cgabspath);
> > > + return ret;
> > > }
> > >
> > > int lxc_freeze(const char *name, const char *lxcpath)
> > > @@ -146,19 +143,16 @@ int lxc_unfreeze(const char *name, const
> > > char *lxcpath) return freeze_unfreeze(name, 0, lxcpath);
> > > }
> > >
> > > -int lxc_unfreeze_bypath(const char *cgpath)
> > > +int lxc_unfreeze_bypath(const char *cgrelpath)
> > > {
> > > - char *nsgroup = NULL;
> > > + char *cgabspath;
> > > int ret;
> > > -
> > > - ret = cgroup_path_get(&nsgroup, "freezer", cgpath);
> > > - if (ret)
> > > - goto fail;
> > >
> > > - return do_unfreeze(nsgroup, 0, NULL, NULL);
> > > + cgabspath = cgroup_path_get("freezer", cgrelpath);
> > > + if (!cgabspath)
> > > + return -1;
> > >
> > > -fail:
> > > - if (nsgroup)
> > > - free(nsgroup);
> > > - return -1;
> > > + ret = do_unfreeze(cgabspath, 0, NULL, NULL);
> > > + free(cgabspath);
> > > + return ret;
> > > }
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 2934afa..b2e5e36 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -570,7 +570,7 @@ static bool create_container_dir(struct
> > > lxc_container *c)
> > > * for ->create, argv contains the arguments to pass to the
> > > template,
> > > * terminated by NULL. If no arguments, you can just pass NULL.
> > > */
> > > -static bool lxcapi_create(struct lxc_container *c, char *t, char
> > > *const argv[]) +static bool lxcapi_create(struct lxc_container
> > > *c, const char *t, char *const argv[]) {
> > > bool bret = false;
> > > pid_t pid;
> > > @@ -636,7 +636,7 @@ static bool lxcapi_create(struct
> > > lxc_container *c, char *t, char *const argv[]) newargv =
> > > malloc(nargs * sizeof(*newargv)); if (!newargv)
> > > exit(1);
> > > - newargv[0] = t;
> > > + newargv[0] = (char *)t;
> >
> > You're typecasting const char* to char*?
> >
> > I agree with making the template arg const char *, but then if you
> > need to do this, it's better to make newargv a char *const (or
> > whatever :) right?
>
> I went ahead and pushed as is (the compile didn't complain) for now.
First off, sorry I should've split out making the arg const as a
separate change.
The problem with trying to make newargv be const char **newargv (which
would make it so we don't have to cast t) is that then we'd have an even
uglier cast when passing it to execv().
I don't understand why argv in execv() is char *const argv[]. Good old
cdecl explain says this is "declare argv as array of const pointer to
char" which I guess means execv() won't change a pointer in the array
but might change the chars pointed to (although I don't think it
actually does). I don't know why its not prototyped as const char *const
argv[].
>
> thanks,
> -serge
More information about the lxc-devel
mailing list