review of the backupkey protocol implementation

Matthieu Patou mat at
Sun Nov 21 07:40:22 MST 2010

On 12/10/2010 11:24, Stefan (metze) Metzmacher wrote:
> Am 12.10.2010 02:06, schrieb Andrew Bartlett:
>> On Tue, 2010-10-05 at 10:43 +0400, Matthieu Patou wrote:
>>> Hello,
>>> I would like to have a code review of my implementation of the backupkey
>>> protocol.
>>> The whole stuff is working correctly now and pass make test but I think
>>> it would be great to have a review just in case there is some code smell.
>>> It's here:
> Please fix the indentation in a lot of places, e.g. the backupkey.idl
> And remove unused stuff.
> Why do we need bkrp_BackupKey_debug? If we really need that it should
> have the 'noopnum'
> property.
> Please create a separate commit that removes the old
> protected_storage.idl and related stuff.
> I think we also don't need the ndr_backupkey_print.[ch] files they can
> be merged into ndr_backupkey.[ch]
> What is BIGNUM ? the '*' of pointers should be next to the variable and
> not the type (in a lot of places)
> The '{' of functions should be in a new line.
> char *addr = "unknown"; should be 'const char *addr = "unknown"'
> You don't need sub_ctx and explicit talloc_free/unlink calls, mem_ctx
> is already the temporary memory context, which is cleaned up after the call.
> And please go through each single line and fix the identation and coding
> style,
> it's really not that hard...

This should be fixed in the current version of this branch on my private 

Can I have your final comment before I push it to autobuild ?

Matthieu Patou
Samba Team
Private repo;a=summary

More information about the samba-technical mailing list