On Sun, Nov 24, 2013 at 08:25:17PM +0100, Pavel Reichl wrote:
> On Fri, 2013-11-22 at 16:46 +0100, Jakub Hrozek wrote:
> > On Wed, Nov 20, 2013 at 01:52:42PM +0100, Pavel Reichl wrote:
> > > New patch hopefully addressing all mentioned issues is attached.
> > > 
> > > PR
> > > 
> > 
> > Hi Pavel,
> > 
> > sorry for the late review.
> > 
> > The patch works fine and mostly looks good, I'm just wondering about one
> > change. I wonder if this block:
> > 
> > > +        if (ret == ENOENT) {
> > > +            /* sss specific error denoting missing configuration file */
> > > +            ret = ERR_MISSING_CONF;
> > > +        }
> > 
> > Would be better placed in the confdb_init_db() function after the
> > sss_ini_config_file_open() fails with ENOENT? Not 100% sure what would
> > look better though.
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel@lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 
> Hi Jakub,
> 
> I think you are right because it makes more obvious which function is
> supposed to fail with ENOENT. Improved patch is attached.
> 
> PR
> 

The code works fine and looks good to me.

ACK.

Congratulations for going through the review process for the first time!
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to