-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 09/18/2009 04:29 PM, Jakub Hrozek wrote:
On 09/18/2009 04:26 PM, Stephen Gallagher wrote:
> usermod(), useradd(), etc.: res should be a talloc child of req. (Also,
> in the current implementation, if user_mod_send returned NULL for
> ENOMEM, you would leak res)
> user_mod_send(), group_mod_send: subreq should be a talloc child of req.
> If for some reason we cancelled the request for usermod, we want to make
> sure that its subrequests aren't continuing.
These two I agree with. Thanks for spotting these.
> There's a lot of code duplication in user_mod_*. Is it possible to code
> this in such a way that you only have one common callback that processes
> the results and kicks off another request if needed?
Yes, but please see Simo's comments[1] to the earlier patches. It seems
that there are different views on code readability (even with
duplication) versus trying not to duplicate code at all, which is what I
tried to do earlier, esp:
<cite>
I know some of the functions (like user_mod_cont()) seem to save a bit
of duplication and makes the code slightly smaller, but this is an
optimization that kills readability and doesn't really save much.
Readability is much more important when it comes to tevent_req stuff.
</cite>
So, I very much appreciate the review, but to be honest, I'd like to
avoid rewriting the code back and forth until I know what is the
approach that would be welcome by all parties.
Thank you for the review,
Jakub
[1]
https://fedorahosted.org/pipermail/sssd-devel/2009-September/000521.html
You know, put that way, I guess I agree with this approach. I'm just
generally averse to having duplicated code because it means multiple
places to change if we have to modify it. But if it's a conscious
decision made to improve readability, I'm behind it.
Fix that talloc parentage and this will be an Ack.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
- --
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEYEARECAAYFAkq3VPwACgkQeiVVYja6o6MvYQCghGcfs2A+epmIDlUCokaPKShL
QqoAnjDsEP1AFCx1YurnbIl5BzKgWbO8
=euoD
-----END PGP SIGNATURE-----