[SSSD] NSS: Add config option to expand homedir template format

Jakub Hrozek jhrozek at redhat.com
Sun Jun 1 17:23:35 UTC 2014


On Wed, May 28, 2014 at 06:22:05PM +0200, Lukas Slebodnik wrote:
> On (27/05/14 16:32), Jakub Hrozek wrote:
> >On Tue, May 27, 2014 at 01:03:42PM +0200, Pavel Reichl wrote:
> >> On Tue, 2014-05-27 at 11:24 +0200, Lukas Slebodnik wrote:
> >> > O
> >> > >The fact of passing pointer to the same area in memory to 2 separate
> >> > >arguments of sss_parse_name() is what I called potential source of bugs.
> >> > >You said "It seems strange to me" so I hope you know what I mean.
> >> > It is strange, but it isn't wrong.
> >> >    * orig_name refers to old string
> >> >    * homedir_ctx->username will refer to new string.
> >> > I need to use old string in debug message if function fails.
> >> I missed that. 
> >> 
> >> I did some testing and all seems to be working, so ACK to all patches.
> >> 
> >
> >In the third patch, you need to add the file
> >src/man/include/override_homedir.xml into src/man/po/po4a.cfg to make sure
> You ment homedir_substring.xml

Yes :)

> 
> >it's processed for translations.
> >
> Added
> 
> >Can you ask some native English speaker to check the contents of the
> >text added?
> >
> >As a side note, it would be nice to treat any refactoring as an
> >opportunity for adding more unit tests. Neither expand_homedir_template
> >nor sss_parse_name_const have any tests.
> The test for expand_homedir_template is in separate commit,
> because patches with refactoring are complicated enough.
> 
> LS

Thanks for the unit test!

I don't have any other comments about functionality or code, just please
amend the man pages as Stephen suggested.

One more improvement might be that you don't have to allocate the
homedir_ctx most of the time, since the functions that consume them are
synchronous. You can just allocate them on stack:
    struct sss_homedir_ctx hctx;
    hctx.name = name;

Especially in responder we already do too many allocations..



More information about the sssd-devel mailing list