[lxc-devel] [PATCH] Use on_path to find init.lxc

[Serge Hallyn]

Quoting Stéphane Graber (stgraber at ubuntu.com):
> On Fri, Mar 28, 2014 at 09:22:07PM -0500, Serge Hallyn wrote:
> > Quoting Stéphane Graber (stgraber at ubuntu.com):
> > > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> > 
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> > 
> > (just one bit of whining below)
> > 
> > > ---
> > >  src/lxc/conf.c    | 19 ++++++++++++++++---
> > >  src/lxc/execute.c | 24 ++++++++++++++++++++++--
> > >  src/lxc/utils.c   | 10 +++++-----
> > >  src/lxc/utils.h   |  2 +-
> > >  4 files changed, 44 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index e34e034..06235fb 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -3197,10 +3197,23 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
> > >  {
> > >  	struct lxc_list *iterator;
> > >  	struct id_map *map;
> > > -	int ret = 0;
> > > +	int ret = 0, use_shadow = 0;
> > >  	enum idtype type;
> > > -	char *buf = NULL, *pos;
> > > -	int use_shadow = (on_path("newuidmap") && on_path("newgidmap"));
> > > +	char *buf = NULL, *pos, *cmdpath = NULL;
> > > +
> > > +	cmdpath = on_path("newuidmap");
> > > +	if (cmdpath) {
> > > +		use_shadow = 1;
> > > +		free(cmdpath);
> > > +	}
> > > +
> > > +	if (!use_shadow) {
> > > +		cmdpath = on_path("newgidmap");
> > > +		if (cmdpath) {
> > > +			use_shadow = 1;
> > > +			free(cmdpath);
> > > +		}
> > > +	}
> > >  
> > >  	if (!use_shadow && geteuid()) {
> > >  		ERROR("Missing newuidmap/newgidmap");
> > > diff --git a/src/lxc/execute.c b/src/lxc/execute.c
> > > index 9a84131..651c47d 100644
> > > --- a/src/lxc/execute.c
> > > +++ b/src/lxc/execute.c
> > > @@ -30,6 +30,7 @@
> > >  #include "conf.h"
> > >  #include "log.h"
> > >  #include "start.h"
> > > +#include "utils.h"
> > >  
> > >  lxc_log_define(lxc_execute, lxc_start);
> > >  
> > > @@ -43,9 +44,27 @@ struct execute_args {
> > >   */
> > >  static char *choose_init(void)
> > >  {
> > > -	char *retv = malloc(PATH_MAX);
> > > -	int ret;
> > > +	char *retv = NULL;
> > > +	int ret, env_set = 0;
> > >  	struct stat mystat;
> > > +
> > > +	if (!getenv("PATH")) {
> > > +		if (setenv("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", 0))
> > 
> > I don't know if standards say otherwise, but personally I don't like
> > having it check /usr/local before /.  Often there's cruft in /usr/local
> > from development attempts.
> 
> I based it on the standard value for Debian/Ubuntu systems which I
> suspect most other distros follow.
> 
> I agree that cruft in /usr/local is a problem but at the same time, when
> used properly, it's also a nice way to override or wrap the real thing
> without modifying / or /usr...

All right, agreed, I'll withdraw my snarky objection :)