Hi Andrew!
Am Dienstag, den 19.02.2008, 09:58 -0600 schrieb Andrew Kroeger:
> The attached patches implement recursive registry deletes as discussed
> in bz #5178, and with Jelmer via IRC.
>> In addition the the recursive delete implementation, I have also updated
> the registry tests to work correctly with the recursive delete.
>> I would appreciate if someone could review my patches and let me know if
> there are any issues I need to address with them.
Thanks! Overall, these changes look ok. Some minor comments:
Rather than ifdeffing out some tests, I would rather see the rpc-winreg
test be converted to the new torture API and having the individual tests
marked as known failing. AFAIK these tests pass against Windows, so it
would be a shame to disable them against Windows as well.
Please don't move functions within the file unless there's a reason for
it. It makes it harder to review.
The gotos in the recursive ldb delete function seem to make the code
more rather than less complex and they each only save one line. Any
chance you can remove at least one of the two labels?
> P.S. - The attached patches should also be in the v4-0-registry branch
> of my Git repo at git://git.id10ts.net/samba.git.>> P.P.S - I'm not sure as to the patch submission protocol with Git - is
> it OK to just point to the patches in my repo, or is it better to keep
> sending patches to samba-technical to maintain an audit trail of what I
> am actually submitting?
Having the patches applied to the mailing list post is nice because it
makes it easier to review and comment in-line where necessary.
Cheers,
Jelmer
--
Jelmer Vernooij <jelmer at samba.org> - http://samba.org/~jelmer/
Jabber: jelmer at jabber.fsfe.org