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