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