[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
Re: changes to distributed collect.c overlay
On Sat, Sep 6, 2008 at 4:01 AM, Howard Chu <hyc@symas.com> wrote:
> Pretty good. Just a couple points - it's not necessary to strdup c->argv[]
> before processing it. Since this overlay is the final consumer of the argv,
> there's no reason to preserve it. Trash it at will. strtok should probably
> be used instead of strtok_r in your config code since strtok_r is not
> universal and the code is single-threaded already.
>
> Also, strcat/strncat are never to be used, use lutil_strcopy/strncopy
> instead.
>
> ci_ad_len should probably be ci_ad_num; "_len" is always used for the length
> of strings, "_num" to count anything else.
Have made suggested changes, thanks :)
New diff is attached...
Cheers
Brett
Index: configure.in
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/configure.in,v
retrieving revision 1.666
diff -u -r1.666 configure.in
--- configure.in 30 Aug 2008 22:18:08 -0000 1.666
+++ configure.in 6 Sep 2008 06:07:12 -0000
@@ -340,6 +340,7 @@
memberof \
ppolicy \
proxycache \
+ collect \
refint \
retcode \
rwm \
@@ -372,6 +373,8 @@
no, [no yes mod], ol_enable_overlays)
OL_ARG_ENABLE(proxycache,[ --enable-proxycache Proxy Cache overlay],
no, [no yes mod], ol_enable_overlays)
+OL_ARG_ENABLE(collect,[ --enable-collect Collect overlay],
+ no, [no yes mod], ol_enable_overlays)
OL_ARG_ENABLE(refint,[ --enable-refint Referential Integrity overlay],
no, [no yes mod], ol_enable_overlays)
OL_ARG_ENABLE(retcode,[ --enable-retcode Return Code testing overlay],
@@ -2842,6 +2845,18 @@
AC_DEFINE_UNQUOTED(SLAPD_OVER_PROXYCACHE,$MFLAG,[define for Proxy Cache overlay])
fi
+if test "$ol_enable_collect" != no ; then
+ BUILD_COLLECT=$ol_enable_collect
+ if test "$ol_enable_collect" = mod ; then
+ MFLAG=SLAPD_MOD_DYNAMIC
+ SLAPD_DYNAMIC_OVERLAYS="$SLAPD_DYNAMIC_OVERLAYS collect.la"
+ else
+ MFLAG=SLAPD_MOD_STATIC
+ SLAPD_STATIC_OVERLAYS="$SLAPD_STATIC_OVERLAYS collect.o"
+ fi
+ AC_DEFINE_UNQUOTED(SLAPD_OVER_COLLECT,$MFLAG,[define for Collect overlay])
+fi
+
if test "$ol_enable_refint" != no ; then
BUILD_REFINT=$ol_enable_refint
if test "$ol_enable_refint" = mod ; then
@@ -3004,6 +3019,7 @@
AC_SUBST(BUILD_MEMBEROF)
AC_SUBST(BUILD_PPOLICY)
AC_SUBST(BUILD_PROXYCACHE)
+ AC_SUBST(BUILD_COLLECT)
AC_SUBST(BUILD_REFINT)
AC_SUBST(BUILD_RETCODE)
AC_SUBST(BUILD_RWM)
Index: include/portable.hin
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/include/portable.hin,v
retrieving revision 1.42
diff -u -r1.42 portable.hin
--- include/portable.hin 26 Aug 2008 19:48:17 -0000 1.42
+++ include/portable.hin 6 Sep 2008 06:07:12 -0000
@@ -978,6 +978,9 @@
/* define for Proxy Cache overlay */
#undef SLAPD_OVER_PROXYCACHE
+/* define for Collect overlay */
+#undef SLAPD_OVER_COLLECT
+
/* define for Referential Integrity overlay */
#undef SLAPD_OVER_REFINT
Index: servers/slapd/overlays/Makefile.in
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/overlays/Makefile.in,v
retrieving revision 1.49
diff -u -r1.49 Makefile.in
--- servers/slapd/overlays/Makefile.in 7 Jan 2008 23:20:14 -0000 1.49
+++ servers/slapd/overlays/Makefile.in 6 Sep 2008 06:07:13 -0000
@@ -22,6 +22,7 @@
dynlist.c \
memberof.c \
pcache.c \
+ collect.c \
ppolicy.c \
refint.c \
retcode.c \
@@ -83,6 +84,9 @@
pcache.la : pcache.lo
$(LTLINK_MOD) -module -o $@ pcache.lo version.lo $(LINK_LIBS)
+collect.la : collect.lo
+ $(LTLINK_MOD) -module -o $@ collect.lo version.lo $(LINK_LIBS)
+
ppolicy.la : ppolicy.lo
$(LTLINK_MOD) -module -o $@ ppolicy.lo version.lo $(LINK_LIBS) $(MODULES_LIBS)
Index: servers/slapd/overlays/collect.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/overlays/collect.c,v
retrieving revision 1.9
diff -u -r1.9 collect.c
--- servers/slapd/overlays/collect.c 7 Jan 2008 23:20:14 -0000 1.9
+++ servers/slapd/overlays/collect.c 6 Sep 2008 06:07:13 -0000
@@ -43,14 +43,15 @@
typedef struct collect_info {
struct collect_info *ci_next;
struct berval ci_dn;
- AttributeDescription *ci_ad;
+ int ci_ad_num;
+ AttributeDescription *ci_ad[];
} collect_info;
static int
collect_cf( ConfigArgs *c )
{
slap_overinst *on = (slap_overinst *)c->bi;
- int rc = 1;
+ int rc = 1, idx;
switch( c->op ) {
case SLAP_CONFIG_EMIT:
@@ -60,11 +61,30 @@
struct berval bv;
int len;
- bv.bv_len = ci->ci_dn.bv_len + STRLENOF("\"\" ") +
- ci->ci_ad->ad_cname.bv_len;
+ // calculate the length & malloc memory
+ bv.bv_len = ci->ci_dn.bv_len + STRLENOF("\"\" ");
+ for (idx=0; idx<ci->ci_ad_num; idx++) {
+ bv.bv_len += ci->ci_ad[idx]->ad_cname.bv_len;
+ if (idx<(ci->ci_ad_num-1)) {
+ bv.bv_len++;
+ }
+ }
bv.bv_val = ch_malloc( bv.bv_len + 1 );
- len = snprintf( bv.bv_val, bv.bv_len + 1, "\"%s\" %s",
- ci->ci_dn.bv_val, ci->ci_ad->ad_cname.bv_val );
+
+ // copy the value and update len
+ len = snprintf( bv.bv_val, bv.bv_len + 1, "\"%s\" ",
+ ci->ci_dn.bv_val);
+ for (idx=0; idx<ci->ci_ad_num; idx++) {
+ lutil_strcopy( bv.bv_val + strlen(bv.bv_val),
+ ci->ci_ad[idx]->ad_cname.bv_val,
+ bv.bv_len);
+ len += ci->ci_ad[idx]->ad_cname.bv_len;
+ if (idx<(ci->ci_ad_num-1)) {
+ bv.bv_val[len++]=',';
+ bv.bv_val[len]='\0';
+ }
+ }
+
assert( len == bv.bv_len );
ber_bvarray_add( &c->rvalue_vals, &bv );
rc = 0;
@@ -98,8 +118,21 @@
collect_info *ci;
struct berval bv, dn;
const char *text;
- AttributeDescription *ad = NULL;
+ int idx, count=0;
+ char *arg;
+ /* count delimiters in attribute argument */
+ arg = strtok(c->argv[2], ",");
+ while (arg!=NULL) {
+ count++;
+ arg = strtok(NULL, ",");
+ }
+
+ /* allocate config info with room for attribute array */
+ ci = ch_malloc( sizeof( collect_info ) +
+ ( sizeof (AttributeDescription *) * (count + 1)));
+
+ /* validate and normalize dn */
ber_str2bv( c->argv[1], 0, 0, &bv );
if ( dnNormalize( 0, NULL, NULL, &bv, &dn, NULL ) ) {
snprintf( c->cr_msg, sizeof( c->cr_msg ), "%s invalid DN: \"%s\"",
@@ -108,20 +141,34 @@
"%s: %s\n", c->log, c->cr_msg, 0 );
return ARG_BAD_CONF;
}
- if ( slap_str2ad( c->argv[2], &ad, &text ) ) {
- snprintf( c->cr_msg, sizeof( c->cr_msg ), "%s attribute description unknown: \"%s\"",
- c->argv[0], c->argv[2] );
- Debug( LDAP_DEBUG_CONFIG|LDAP_DEBUG_NONE,
- "%s: %s\n", c->log, c->cr_msg, 0 );
- return ARG_BAD_CONF;
+
+ /* load attribute description for attribute list */
+ arg = c->argv[2];
+ for( idx=0; idx<count; idx++) {
+ ci->ci_ad[idx] = NULL;
+
+ if ( slap_str2ad( arg, &ci->ci_ad[idx], &text ) ) {
+ snprintf( c->cr_msg, sizeof( c->cr_msg ),
+ "%s attribute description unknown: \"%s\"",
+ c->argv[0], arg);
+ Debug( LDAP_DEBUG_CONFIG|LDAP_DEBUG_NONE,
+ "%s: %s\n", c->log, c->cr_msg, 0 );
+ return ARG_BAD_CONF;
+ }
+ while(*arg!='\0') {
+ arg++; // skip to end of argument
+ }
+ if (idx<count-1) {
+ arg++; // skip inner delimiters
+ }
}
/* The on->on_bi.bi_private pointer can be used for
* anything this instance of the overlay needs.
*/
- ci = ch_malloc( sizeof( collect_info ));
- ci->ci_ad = ad;
- ci->ci_dn = dn;
+ ci->ci_ad[count] = NULL;
+ ci->ci_ad_num = count;
+ ci->ci_dn = dn;
ci->ci_next = on->on_bi.bi_private;
on->on_bi.bi_private = ci;
rc = 0;
@@ -167,6 +214,50 @@
}
static int
+collect_modify( Operation *op, SlapReply *rs)
+{
+ slap_overinst *on = (slap_overinst *) op->o_bd->bd_info;
+ collect_info *ci = on->on_bi.bi_private;
+ Modifications *ml;
+ char errMsg[100];
+ int rc, idx;
+
+ for ( ml = op->orm_modlist; ml != NULL; ml = ml->sml_next) {
+ for (; ci; ci=ci->ci_next ) {
+
+ /* Is this entry an ancestor of this collectinfo ? */
+ if (!dnIsSuffix(&op->o_req_ndn, &ci->ci_dn)) {
+ /* this collectinfo does not match */
+ continue;
+ }
+
+ /* Is this entry the same as the template DN ? */
+ if ( strcmp(op->o_req_ndn.bv_val, ci->ci_dn.bv_val) == 0) {
+ /* dont apply change collect-ed attributes */
+ continue;
+ }
+
+ /* check for collect attributes - disallow modify if present */
+ for(idx=0; idx<ci->ci_ad_num; idx++) {
+ if (strcmp(ml->sml_desc->ad_cname.bv_val,
+ ci->ci_ad[idx]->ad_cname.bv_val) == 0) {
+ rs->sr_err = LDAP_UNWILLING_TO_PERFORM;
+ snprintf( errMsg, sizeof( errMsg ),
+ "cannot change virtual attribute '%s'",
+ ci->ci_ad[idx]->ad_cname.bv_val);
+ rs->sr_text = errMsg;
+ send_ldap_result( op, rs );
+ return rs->sr_err;
+ }
+ }
+ }
+
+ }
+
+ return SLAP_CB_CONTINUE;
+}
+
+static int
collect_response( Operation *op, SlapReply *rs )
{
slap_overinst *on = (slap_overinst *) op->o_bd->bd_info;
@@ -181,35 +272,51 @@
op->o_bd->bd_info = (BackendInfo *)on->on_info;
for (; ci; ci=ci->ci_next ) {
- BerVarray vals = NULL;
+ int idx=0;
+
+ /* Is this entry an ancestor of this collectinfo ? */
+ if (!dnIsSuffix(&rs->sr_entry->e_nname, &ci->ci_dn)) {
+ /* collectinfo does not match */
+ continue;
+ }
- /* Is our configured entry an ancestor of this one? */
- if ( !dnIsSuffix( &rs->sr_entry->e_nname, &ci->ci_dn ))
+ /* Is this entry the same as the template DN ? */
+ if ( strcmp(rs->sr_entry->e_nname.bv_val, ci->ci_dn.bv_val) == 0) {
+ /* dont apply change to parent */
continue;
+ }
- /* Extract the values of the desired attribute from
- * the ancestor entry
- */
- rc = backend_attribute( op, NULL, &ci->ci_dn, ci->ci_ad, &vals, ACL_READ );
-
- /* If there are any values, merge them into the
- * current entry
- */
- if ( vals ) {
- /* The current entry may live in a cache, so
- * don't modify it directly. Make a copy and
- * work with that instead.
- */
- if ( !( rs->sr_flags & REP_ENTRY_MODIFIABLE )) {
- rs->sr_entry = entry_dup( rs->sr_entry );
- rs->sr_flags |= REP_ENTRY_MODIFIABLE |
- REP_ENTRY_MUSTBEFREED;
+ /* The current entry may live in a cache, so
+ * don't modify it directly. Make a copy and
+ * work with that instead.
+ */
+ if ( !( rs->sr_flags & REP_ENTRY_MODIFIABLE )) {
+ rs->sr_entry = entry_dup( rs->sr_entry );
+ rs->sr_flags |= REP_ENTRY_MODIFIABLE |
+ REP_ENTRY_MUSTBEFREED;
+ }
+
+ /* Loop for each attribute in this collectinfo */
+ for(idx=0; idx<ci->ci_ad_num; idx++) {
+ BerVarray vals = NULL;
+
+ /* Extract the values of the desired attribute from
+ * the ancestor entry */
+ rc = backend_attribute( op, NULL, &ci->ci_dn,
+ ci->ci_ad[idx], &vals, ACL_READ );
+
+ /* If there are any values, merge them into the
+ * current search result
+ */
+ if ( vals ) {
+ attr_merge( rs->sr_entry, ci->ci_ad[idx],
+ vals, NULL );
+ ber_bvarray_free_x( vals, op->o_tmpmemctx );
}
- attr_merge( rs->sr_entry, ci->ci_ad, vals, NULL );
- ber_bvarray_free_x( vals, op->o_tmpmemctx );
}
}
- }
+ }
+
/* Default is to just fall through to the normal processing */
return SLAP_CB_CONTINUE;
}
@@ -221,6 +328,7 @@
collect.on_bi.bi_type = "collect";
collect.on_bi.bi_db_destroy = collect_destroy;
+ collect.on_bi.bi_op_modify = collect_modify;
collect.on_response = collect_response;
collect.on_bi.bi_cf_ocs = collectocs;