[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: commit: ldap/servers/slapd/back-relay op.c



ando@OpenLDAP.org writes:
> Modified Files:
> 	op.c  1.24 -> 1.25
> rework back-relay internals along Hallvard's suggestions (ITS#5328)

Looking closer...

The code to handle RB_REFERRAL is unused, since RB_REFERRAL is never
passed.

Nitpicks:

RB_UNWILLING (and its code) can be factored as
(LDAP_UNWILLING_TO_PERFORM | RB_ERR | <set sr_text>).
Eliminates the RB_UNWILLING case in relay_back_select_backend().

The code looks clearer with the RB_SEND-handling inside the
blocks handling RB_ERR.  (The code supports sending a result without
setting sr_err first, which looks strange.  If you used RB_SEND without
RB_ERR/RB_REFERRAL.)

I'll commit something like this for the two latter, unless you have
other plans.  (And maybe rename RB_UNWILLING* to RB_UNAVAIL* since it's
the <set sr_text to "unavailable in this context> part which differs
from what other flags do.)

Index: op.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-relay/op.c,v
retrieving revision 1.26
diff -u -2 -r1.26 op.c
--- op.c	16 Jan 2008 21:00:57 -0000	1.26
+++ op.c	18 Jan 2008 14:15:12 -0000
@@ -28,7 +28,8 @@
 #define	RB_ERR_MASK		(0x00FFU)
 #define RB_ERR			(0x1000U)
-#define RB_UNWILLING		(0x2000U)
+#define RB_UNAVAIL_FLAG		(0x2000U)
 #define RB_REFERRAL		(0x4000U)
 #define RB_SEND			(0x8000U)
+#define RB_UNWILLING	(LDAP_UNWILLING_TO_PERFORM|RB_ERR|RB_UNAVAIL_FLAG)
 #define	RB_UNWILLING_SEND	(RB_UNWILLING|RB_SEND)
 #define	RB_REFERRAL_SEND	(RB_REFERRAL|RB_SEND)
@@ -76,13 +77,9 @@
 				"%s: back-relay for DN=\"%s\" would call self.\n",
 				op->o_log_prefix, op->o_req_dn.bv_val, 0 );
-			if ( fail_mode & RB_UNWILLING ) {
-				rs->sr_err = LDAP_UNWILLING_TO_PERFORM;
-
-			} else if ( fail_mode & RB_ERR ) {
+			if ( fail_mode & RB_ERR ) {
 				rs->sr_err = rc;
-			}
-
-			if ( fail_mode & RB_SEND ) {
-				send_ldap_result( op, rs );
+				if ( fail_mode & RB_SEND ) {
+					send_ldap_result( op, rs );
+				}
 			}
 
@@ -148,10 +145,7 @@
 		}
 
-	} else {
-		if ( fail_mode & RB_ERR ) {
-			rs->sr_err = rc;
-
-		} else if ( fail_mode & RB_UNWILLING ) {
-			rc = rs->sr_err = LDAP_UNWILLING_TO_PERFORM;
+	} else if ( fail_mode & RB_ERR ) {
+		rs->sr_err = rc;
+		if ( fail_mode & RB_UNAVAIL_FLAG ) {
 			rs->sr_text = "operation not supported within naming context";
 		}

-- 
Regards,
Hallvard