[lxc-devel] [PATCH] rcfile shouldn't get set if the attempt to load a config file fails

[S.Çağlar Onur]

Hey Serge,

To be honest I wasn't thinking about parse errors while sending the patch.
Maybe we can handle the non-existent file issue with something like that

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 8fe1541..96f9122 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -1422,6 +1422,11 @@ int lxc_config_readline(char *buffer, struct
lxc_conf *conf)

 int lxc_config_read(const char *file, struct lxc_conf *conf)
 {
+       /* make sure file exists and reabable by us */
+       if( access(file, R_OK) == -1 ) {
+               return -1;
+       }
+
        /* Catch only the top level config file name in the structure */
        if( ! conf->rcfile ) {
                conf->rcfile = strdup( file );

and address parse errors differently?

Cheers,



On Wed, Mar 27, 2013 at 4:09 PM, Serge Hallyn <serge.hallyn at ubuntu.com>wrote:

> Quoting S.Çağlar Onur (caglar at 10ur.org):
> > From: "S.Çağlar Onur" <caglar at 10ur.org>
> >
> > Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
>
> The only downside I see is that now the callbacks can be called
> with conf->rcfile NULL when it otherwise might be set to strdup(file).
>
> This gets tricky because the file being read could have a #include
> which will trigger lxc_config_read() again.  So now the recursive call
> will end up setting rcfile.
>
> This gets tricky but I think the best thing to do is
>
> if (!conf->rcfile) {
>         was_unset = true;
>         conf->rcfile = strdup(file);
> }
> ret = lxc_file_for_each_line(file, parse_line, conf);
> if (ret != 0 && was_unset) {
>         free(conf->rcfile;
>         conf->rcfile = NULL;
> }
>
> Maybe you have a better idea though.
>
> > ---
> >  src/lxc/confile.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index 8fe1541..16b8594 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -1422,11 +1422,13 @@ int lxc_config_readline(char *buffer, struct
> lxc_conf *conf)
> >
> >  int lxc_config_read(const char *file, struct lxc_conf *conf)
> >  {
> > +     int ret = lxc_file_for_each_line(file, parse_line, conf);
> > +
> >       /* Catch only the top level config file name in the structure */
> > -     if( ! conf->rcfile ) {
> > +     if( ret == 0 && ! conf->rcfile ) {
> >               conf->rcfile = strdup( file );
> >       }
> > -     return lxc_file_for_each_line(file, parse_line, conf);
> > +     return ret;
> >  }
> >
> >  int lxc_config_define_add(struct lxc_list *defines, char* arg)
> > --
> > 1.7.10.4
> >
> >
> >
> ------------------------------------------------------------------------------
> > Own the Future-Intel® Level Up Game Demo Contest 2013
> > Rise to greatness in Intel's independent game demo contest.
> > Compete for recognition, cash, and the chance to get your game
> > on Steam. $5K grand prize plus 10 genre and skill prizes.
> > Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
>



-- 
S.Çağlar Onur <caglar at 10ur.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130327/58ddc980/attachment.html>