PATCHv5: adjust 'net ads add keytab' for windows SPN(s) & add new 'net ads setspn' subcommand

Noel Power nopower at
Wed Feb 28 11:36:13 UTC 2018

Thanks Jeremy for having a look, I have adjusted the patch

summary of changes:
  * bulk 'principle' -> 'principal'
  * patch2: redundant else clause removed
  * patch3: missing talloc return checks added
  * patch4:
>> +
>> +static bool ads_set_machine_account_spns(TALLOC_CTX *ctx,
> Return ADS_STATUS here, otherwise you're losing a
> error code that might be useful higher up.
This is a local static function, the only caller (in this module)
doesn't return an ADS_STATUS so it will never be passed up anywhere so I
didn't change it, hope that is ok.  Do you still want me to change it ?

  * patch5: redundant talloc_stackframe return checks removed,
'ads_setspn_list' return changed to 'bool'
  * patch6: redundant talloc_stackframe return checks removed,
'ads_setspn_add' return changed to 'bool'

> ads_setspn_add returns either 0 or -1. Why isn't this
> a bool ? 
For consistency I just followed the existing code where presumably for
simplicity the implementation functions called by cmdline processing
(int (*fn)(struct net_context *c, int argc, const char **argv) handler
generally call a helper function with the same return type so they can
pass back the results without conversion.

Anyway I've changed all the setspn functions to use bool now
> I'm not 100% convinced on the |= idiom for
> a return value (I know the ldb code uses it a lot).
> Only a nit-pick here really...
>> +		ret |= ads_setspn_add(ads, argv[0], argv[1]);
>> +	} else {
>> +		ret |= ads_setspn_add(ads, lp_netbios_name(), argv[0]);
>> +	}
>> +
>> +done:
>> +	if (ads) {
>> +		ads_destroy(&ads);
>> +	}
>> +	return ret;
>> +}
I've removed these (for setspn subcommands) also

  * patch7: redundant talloc_stackframe return checks removed,
'ads_setspn_delete' return changed to 'bool'

-------------- next part --------------
A non-text attachment was scrubbed...
Name: keytab_setspn_and_testsv5.patch
Type: text/x-patch
Size: 77439 bytes
Desc: not available
URL: <>

More information about the samba-technical mailing list