[PATCH] [s3] set proper ads_keytab_flush() return code on error

swen swen at linux.ibm.com
Wed Nov 21 10:43:02 UTC 2018


Hi Ralph

On Wed, 2018-11-21 at 11:14 +0100, Ralph Böhme via samba-technical
wrote:
> Hi Swen,
> 
> On Wed, Nov 21, 2018 at 11:06:55AM +0100, swen via samba-technical
> wrote:
> > Please review and push if happy.
> 
> oh, good catch, how did you find that?
...by accident. Working on something else in that area.
> 
> > @@ -477,7 +478,7 @@ int ads_keytab_flush(ADS_STRUCT *ads)
> > 	if (!ADS_ERR_OK(aderr)) {
> > 		DEBUG(1, (__location__ ": Error while clearing service
> > "
> > 			  "principal listings in LDAP.\n"));
> > -		goto out;
> > +		ret = -1;
> > 	}
> > 
> > out:
> 
> I guess this should also use 
> 
> 		ret = -1;
>  		goto out;
> 
> otherwise the next person to add code after the if and before the out
> might not 
> notice that there's no goto out.
> 
> What do you think?
Hmm, I personally would have removed it, but have no really strong
feelings about it :-) ....so put it back in.

Cheers Swen
-------------- next part --------------
From c91c597ce800c8ca1f1bec302e1c5d30361a62d6 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Wed, 21 Nov 2018 10:59:31 +0100
Subject: [PATCH] [s3] set proper ads_keytab_flush() return code on error

The return code was left on success when the calls to
ads_get_machine_kvno() or ads_clear_service_principal_names()
failed and the processing had to be aborted.

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 source3/libads/kerberos_keytab.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
index 63c1f2bb369..74c99e0343e 100644
--- a/source3/libads/kerberos_keytab.c
+++ b/source3/libads/kerberos_keytab.c
@@ -457,6 +457,7 @@ int ads_keytab_flush(ADS_STRUCT *ads)
 	if (kvno == -1) {
 		/* -1 indicates a failure */
 		DEBUG(1, (__location__ ": Error determining the kvno.\n"));
+		ret = -1;
 		goto out;
 	}
 
@@ -477,6 +478,7 @@ int ads_keytab_flush(ADS_STRUCT *ads)
 	if (!ADS_ERR_OK(aderr)) {
 		DEBUG(1, (__location__ ": Error while clearing service "
 			  "principal listings in LDAP.\n"));
+		ret = -1;
 		goto out;
 	}
 
-- 
2.17.2



More information about the samba-technical mailing list