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

[Jeremy Allison]

On Wed, Feb 28, 2018 at 11:36:13AM +0000, Noel Power via samba-technical wrote:
> 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'

Thanks for all your hard work Noel:

Reviewed-by: Jeremy Allison <jra at samba.org>

Andreas, can you do a quick re-review and then
I can push for Noel if you're happy ?

Thanks,

	Jeremy.