On Mon, Jun 02, 2014 at 06:42:05PM +0200, Jakub Hrozek wrote:
> On Mon, Jun 02, 2014 at 05:16:05PM +0200, Lukas Slebodnik wrote:
> > On (02/06/14 10:01), Jakub Hrozek wrote:
> > >On Mon, Jun 02, 2014 at 07:26:49AM +0200, Lukas Slebodnik wrote:
> > >> On (01/06/14 19:23), Jakub Hrozek wrote:
> > >> >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.
> > done
> >
> > >> will do after agreement about allocation of homedir_ctx.
> > >> I do not want to send patchset more than once :-).
> > >>
> > >> >
> > >> >One more improvement might be that you don't have to allocate the
> > >> >homedir_ctx most of the time,
> > >> It is just a *one* allocation and reason is to have a zero initialized
> > >> structure. If you really want to avoid one call of talloc_zero I can
> > >> replace it
> > >> with structure allocated on stack and zeroing structure with memset.
> > >
> > >We have a ZERO_STRUCT call precisely for this reason.
> > >
> > done
> >
> > >This patch is targeted for sssd-1-11 and we're close to the 1.11.6
> > >deadline. If you think you can spin up another version quickly, then
> > >please do, otherwise let's clean up the unneeded allocation in 1.12.
> >
> > new version attached
> >
> > LS
>
> Perfect, thank you.
>
> We can decide about ZERO_STRUCT/{ 0 } later, but I'd like to include
> these patches in 1.11.6 and the deadline is looming..
>
> The patches work fine and look good to me.
>
> ACK
Pushed to master:
59af140ef81f6d0f10db9549089998f5e05631cb
ae0a5011e2644eaa482ea1b9e1451eff05c676b9
5cd660aaa885bca95ac3dca660bb77e5786d5f8e
be7eabee6b7eb8def2441bf5de4c6d4950c155bf
Can you also send a version that applies cleanly on top of sssd-1-11 ?
(or point out patches that need backporting before yours)
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
