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

Stephen Gallagher sgallagh at redhat.com
Mon Jun 11 16:45:23 UTC 2012


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!
-------------- 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/20120611/c8396232/attachment.sig>


More information about the sssd-devel mailing list