gmane.linux.kernel.containers.lxc.devel

Re: [PATCH] fix getline(3) memory leaks

Christian Seiler — 2013-05-22T07:16:10

Hi there,


I'm a bit confused, this goes against everything I know about C memory
management:

ANSI C (ANSI X3J11/88-090, May 13, 1988) [1], Section 4.10.3.2:

  If ptr is a null pointer, no action occurs.

ISO C (ISO/IEC 9899:TC3) [2], Section 7.20.3.2:

  If ptr is a null pointer, no action occurs.

POSIX.1-2008 [3], free function definition:

  If ptr is a null pointer, no action shall occur.

free(3) manpage, Linux:

  If ptr is NULL, no operation is performed.

Test case (volatile to make the compiler not optimize it away):

#include <stdlib.h>

int main(int argc, char **argv)
{
  void * volatile ptr = NULL;
  free(ptr);
  return 0;
}

Works fine on my system.

I have yet to see an implementation of free() which does not implement
this. Ok, maybe there is one from ancient times, but any current Linux
library should have no problems with this,  otherwise I would consider
that to be a serious bug. A *lot* of code I've read through in lots of
different open source projects actually relies on this fact.

Re: [PATCH] fix getline(3) memory leaks

Serge Hallyn — 2013-05-22T01:27:30

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

Yeah I've tested before (because I used to want to think glibc would
check for that) and it definately segvs.  I'll push this patch with
that added.

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] fix getline(3) memory leaks

Dwight Engen — 2013-05-21T22:59:29

On Tue, 21 May 2013 17:07:54 -0500
Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org> wrote:


Hmm, I guess that would be good defensive programming but it seemed
unlikely to me that getline() would find no lines
in /proc/<pid>/status. line was initialized to NULL, so at worst we'd
free(NULL), which I guess not every free() handles well. If you think
we should put the if in I'm fine with that :)



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] fix getline(3) memory leaks

Serge Hallyn — 2013-05-21T22:07:54

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

does this need to be

if (line)
free(line);
?


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] oracle template: mount /dev/shm as tmpfs

Serge Hallyn — 2013-05-21T21:47:17

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>


I'm acking this anyway, but I'd point out that this looks like it
will turn out as a rootfs which no longer is booted under kvm the
exact same way as in a container.  It'll still boot so really no
big deal...


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] lxc-fedora template. Cleanup for rootfs.

Serge Hallyn — 2013-05-21T21:42:53

Quoting Michael H. Warfield (mhw-BetbSzk+GohWk0Htik3J/w< at >public.gmane.org):

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>

(This'll cause a minor conflict with the create-api patch.  It'll
be trivial to resolve, so long as it isn't *ignored* )







------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] fix build with --enable-tests

Serge Hallyn — 2013-05-21T21:39:37

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

d'oh.  thanks.

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

[PATCH] fix getline(3) memory leaks

Dwight Engen — 2013-05-21T19:36:32

Signed-off-by: Dwight Engen <dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org>
---
 src/lxc/attach.c | 2 +-
 src/lxc/cgroup.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index a7e9074..d8b4915 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
< at >< at > -114,6 +114,7 < at >< at > struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 }
 }
 
+free(line);
 fclose(proc_file);
 
 if (!found) {
< at >< at > -145,7 +146,6 < at >< at > struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 
 out_error:
 free(info);
-free(line);
 return NULL;
 }
 
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 44266de..2ffbb54 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
< at >< at > -503,6 +503,7 < at >< at > static char *get_all_cgroups(void)
 }
 
 out:
+free(line);
 fclose(f);
 return ret;
 }

[RFC] [PATCH] lxc-fedora template - Add --fqdn optionfor utsname.

Michael H. Warfield — 2013-05-21T18:47:18

All,

After comments from Serge, attached is a suggested patch to the Fedora
template to incorporate the FQDN (Fully Qualified Domain Name) of the
container as the utsname in the container configuration and in the host
name of the system.  It adds a "--fqdn" long option to explicitly
specify the value for utsname and hostname.

The logic contains one default behavior change.  If an FQDN is not
specified or is a host simple name, the logic will use the domain name
of the host appended to either the container name or the simple name to
generate an FQDN.

There were some comments on the -users list about determining the name
of a container based on it's host name.  That's never a good idea
anyways because they can be independently changed but this would also
change the default case.  Please looks that over and comment.  Other
container owners might consider a similar modification.

Regards,
Mike

[PATCH] lxc-fedora template. Cleanup for rootfs.

Michael H. Warfield — 2013-05-21T18:17:25

This is just some minor changes in the way the Fedora template is
synthesizing the target rootfs_path.  Currently, the template uses a
path with the container in it twice like this:

/var/lib/lxc/rasputin/rasputin/rootfs

This happens because the container name is already contained in the
"path" and the template appends it a second time.  This changes the
logic to be congruent with other templates such as lxc-arch.  The new
behavior will be to create the rootfs like this:

/var/lib/lxc/rasputin/rootfs

Attached below the jump.

Regards,
Mike

[PATCH] fix build with --enable-tests

Dwight Engen — 2013-05-21T17:30:09

Signed-off-by: Dwight Engen <dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org>
---
 src/lxc/lxccontainer.h |  1 +
 src/tests/cgpath.c     |  3 ++-
 src/tests/clonetest.c  | 10 +++++-----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
index bf3168c..22ccb0b 100644
--- a/src/lxc/lxccontainer.h
+++ b/src/lxc/lxccontainer.h
< at >< at > -103,6 +103,7 < at >< at > struct lxc_container {
  *  then the original backing store's size will be used if possible.  Note this
  *  only applies to the rootfs.  For any other filesystems, the original size
  *  will be duplicated.
+ * < at >hookargs: additional arguments to pass to the clone hook script
  */
 struct lxc_container *(*clone)(struct lxc_container *c, const char *newname,
 const char *lxcpath, int flags, const char *bdevtype,
diff --git a/src/tests/cgpath.c b/src/tests/cgpath.c
index b7f0885..2497e9e 100644
--- a/src/tests/cgpath.c
+++ b/src/tests/cgpath.c
< at >< at > -27,6 +27,7 < at >< at >
 #include <errno.h>
 #include "../lxc/

Re: [PATCH] fix minor gcc 4.7.2 error

Stéphane Graber — 2013-05-21T17:18:20

Oops, forgot to test-build with that last minute change...

Acked-by: Stéphane Graber <stgraber-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>

[PATCH] fix minor gcc 4.7.2 error

Dwight Engen — 2013-05-21T17:15:53

lxccontainer.c:874:4: error: ‘for’ loop initial declarations are only
allowed in C99 mode

Signed-off-by: Dwight Engen <dwight.engen< at >oracle.com>
---
 src/lxc/lxccontainer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 7ce25c6..2934afa 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
< at >< at > -871,7 +871,8 < at >< at > out:
 count++;
 temp = realloc(addresses, count * sizeof(*addresses));
 if (!temp) {
-for (int i = 0; i < count-1; i++)
+int i;
+for (i = 0; i < count-1; i++)
 free(addresses[i]);
 free(addresses);
 return NULL;

[PATCH] oracle template: mount /dev/shm as tmpfs

Dwight Engen — 2013-05-21T15:34:45

sem_open(3) checks that /dev/shm is SHMFS_SUPER_MAGIC. Normally /dev/shm
is mounted in the initramfs created by dracut, but that won't be run for
a container so make sure that rc.sysinit mounts /dev/shm.

Signed-off-by: Dwight Engen <dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org>
---
 templates/lxc-oracle.in | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/templates/lxc-oracle.in b/templates/lxc-oracle.in
index 946956d..70ef632 100644
--- a/templates/lxc-oracle.in
+++ b/templates/lxc-oracle.in
< at >< at > -142,6 +142,17 < at >< at > EOF
         sed -i 's|action $"Setting network parameters|# LXC action $"Setting network parameters|' $container_rootfs/etc/init.d/NetworkManager 2>/dev/null
     fi
 
+    # sem_open(3) checks that /dev/shm is SHMFS_SUPER_MAGIC, so make sure to mount /dev/shm (normally done by dracut initrd) as tmpfs
+    if [ $container_release_major = "4" -o $container_release_major = "5" ]; then
+        echo "mount -t tmpfs tmpfs /dev/shm" >>$container_rootfs/etc/rc.sysinit
+        ech

Re: [PATCH] extend command processor to handle generic data

Serge Hallyn — 2013-05-21T15:05:35

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

Right, thanks.

-serge

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH V2] lxcapi: Add new get_ips() call

Serge Hallyn — 2013-05-21T15:04:04

Quoting Stéphane Graber (stgraber-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org):

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] extend command processor to handle generic data

Dwight Engen — 2013-05-21T14:44:31

On Tue, 21 May 2013 08:18:01 -0500
Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org> wrote:


Creating a container using the API/bindings creates a new c, and
c->lxc_conf contains a parse of the on-disk configuration as it
exists at the time of the lxcContainer() call but may not reflect an
already running c1. I specifically wanted to know the veth pair name of
an already running container, and I think it may be useful to know other
configuration items of a running container which may differ from what
is currently on disk.



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

[PATCH V2] lxcapi: Add new get_ips() call

Stéphane Graber — 2013-05-21T14:41:29

This adds a new get_ips call which takes a family (inet, inet6 or NULL),
a network interface (or NULL for all) and a scope (0 for global) and returns
a char** of all the IPs in the container.

This also adds a matching python3 binding (function result is a tuple) and
deprecates the previous pure-python get_ips() implementation.

WARNING: The python get_ips() call is quite different from the previous
implementation. The timeout argument has been removed, the family names are
slightly different (inet/inet6 vs ipv4/ipv6) and an extra scope parameter
has been added.

Signed-off-by: Stéphane Graber <stgraber< at >ubuntu.com>
---
 src/lxc/lxccontainer.c              | 116 ++++++++++++++++++++++++++++++++++++
 src/lxc/lxccontainer.h              |   1 +
 src/python-lxc/examples/api_test.py |   9 ++-
 src/python-lxc/lxc.c                |  61 +++++++++++++++++++
 src/python-lxc/lxc/__init__.py      |  61 -------------------
 5 files changed, 186 insertions(+), 62 deletions(-)

diff --git a/src/lxc/lxccontainer.c b/src/l

Re: Question about a "real" LXC library

Serge Hallyn — 2013-05-21T13:42:37

Quoting Jon Taylor (dosadi82-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org):

See /usr/include/lxc/lxccontainer.h for the 'real' lxc library, which is
still being developed.  It still has to shell out for some things, like
zfs device handling, and it will always call out to the templates to
fill in new container root filesystems (though you can compile your own
templates as c code if you like).  But many of the shell scripts are
being replaced by c code and exported to lua/python3/go.

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] extend command processor to handle generic data

Serge Hallyn — 2013-05-21T13:18:01

Quoting Dwight Engen (dwight.engen-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org):

Working great on my box - thanks.

Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org>

It seems like a general cleanup, but I'm wondering what your motivation
was for the get-config command?  Programs using the struct
lxc_container API or lua/python3 bindings can after all just do

import lxc
c = lxcContainer('c1', NULL);
c.get_config_item('lxc.rootfs')


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH] lxcapi: Add new get_ips() call

Serge Hallyn — 2013-05-21T04:20:33

Quoting Stéphane Graber (stgraber-GeWIH/nMZzLQT0dZR+AlfA< at >public.gmane.org):

I think you're better off, in this case, doing something like

if (!temp) {
free(addresses[count-2]);
addresses[count-2] = NULL;
} else {
addresses = temp;
addresses[count - 1] = NULL;
}

That, or free everything.  But as it is, you are leaking memory here.

Otherwise, looks good - thanks!

-serge

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may