[SSSD] [PATCH] Per-domain re_expression and full_name_format

Stephen Gallagher sgallagh at redhat.com
Wed Jun 13 01:28:25 UTC 2012


On Mon, 2012-06-11 at 12:45 -0400, Stephen Gallagher wrote:
> On Mon, 2012-06-11 at 17:14 +0200, Stef Walter wrote:
> > Sorry for the delay. Rebased onto master.
> > 
> > On 05/10/2012 02:12 AM, Stephen Gallagher wrote:
> > > Thanks for the patch, you're making great progress here.
> > > 
> > > A few issues remain:
> > > 
> > > sss_parse_name_for_domains():
> > > Please create a tmp_ctx and use that for allocating dmatch and nmatch,
> > > instead of allocating them on memctx. You're effectively leaking memory
> > > in a way that valgrind cannot detect (they're allocated and attached to
> > > memctx, but unreachable). Please use the tmp_ctx and steal the results
> > > onto memctx just prior to returning from the function. Also make sure
> > > that all exit points free tmp_ctx.
> > 
> > Fixed in the attached patch.
> > 
> > > Please prefer to use the 'bool' type for only_name_seen and
> > > only_name_mismatch and 'true' or 'false' as values, instead of zero and
> > > one. It just reads easier.
> > 
> > Makes sense.
> > 
> > > I think only_name_seen is probably unnecessary. You should be able to
> > > just check whether only_name is NULL. You have a guarantee that nmatch
> > > will never be NULL if code == EOK after sss_parse_name. Well, assuming
> > > no bugs in sss_parse_name, but it's not your responsibility in
> > > sss_parse_name_for_domains() to work around such bugs. You can simplify
> > > the mismatch testing considerably with this in mind.
> > > 
> > > Actually, thinking more on this, you should have a guarantee that if
> > > dmatch is NULL and code is EOK, nmatch will always be the same as orig.
> > > Again, if it is not, that would be a bug in sss_parse_name().
> > 
> > But I don't think that's the case. A regular expression can easily
> > produce a name but no domain.
> 
> 
> Stef and I discussed this on IRC. We came to the agreement that it's
> valid (if ugly) for a regular expression to modify the username (such as
> substituting underscores for spaces), so we do need to handle this case.
> So with that in mind, ack to this patch.
> 
> Thanks very much!

Pushed to master.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120612/808ee62d/attachment.sig>


More information about the sssd-devel mailing list