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

Simo Sorce simo at redhat.com
Mon Jun 2 16:11:08 UTC 2014


On Mon, 2014-06-02 at 18:06 +0200, Lukas Slebodnik wrote:
> On (02/06/14 12:01), Simo Sorce wrote:
> >On Mon, 2014-06-02 at 10:01 +0200, 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.
> >> > 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.
> >
> >If it is on the stack, we can simply declare it with a C99 initializer
> >of 0 ?
> >Or is this structure reused (hence needs resetting) more than once ?
> >
> I sent patch with ZERO_STRUCT.
> 
> Feel free to file ticket to get rid of this macro from sssd source code.

Why should we get rid of it ??

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list