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. >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.. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
