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

Stef Walter stefw at gnome.org
Mon Jun 11 15:14:07 UTC 2012


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.

Cheers,

Stef
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-re_expression-and-full_name_format-per-domain-o.patch
Type: text/x-patch
Size: 33926 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120611/1e3c8176/attachment.bin>


More information about the sssd-devel mailing list