Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1290111

Note: Check GER as this implements the extension already.

attachment
0001-Ticket-48363-Support-for-rfc3673-to-return-operation.patch

Hi William, I found a couple of minor issues...

1) I think the year of the copyright would be 2016? ;)
* Copyright (C) 2005 Red Hat, Inc.

2) You defined OID in slapi-plugin.h:
5622 #define LDAP_FEATURE_RFC3673_ALLOPERATIONAL "1.3.6.1.4.1.4203.1.5.1"

The OID is defined in /usr/include/ldap.h.

define LDAP_FEATURE_ALL_OP_ATTRS "1.3.6.1.4.1.4203.1.5.1" / RFC 3673 /

So, I'd recommend to put your define in #if !defined(USE_OPENLDAP) and use the same macro LDAP_FEATURE_ALL_OP_ATTRS for not USE_OPENLDAP case (== MOZLDAP).

3) For some reason, features.c does not include slapi-plugin.h and the server fails to start for me as follows... Isn't it needed?
{{{
Program received signal SIGILL, Illegal instruction.
__GIpthreadrwlock_unlock (rwlock=0x5555557d67a0) at pthread_rwlock_unlock.c:34
34 if (ELIDE_UNLOCK (rwlock->data.__writer == 0

0 __GI___pthread_rwlock_unlock (rwlock=0x5555557d67a0) at pthread_rwlock_unlock.c:34

1 0x00007ffff7b64cfb in slapi_rwlock_unlock (rwlock=0x5555557d67a0)

at ldap/servers/slapd/slapi2nspr.c:254

2 0x00007ffff7aef9af in slapi_get_supported_features_copy (ftroidsp=0x7fffffff7120)

at ldap/servers/slapd/features.c:46

3 0x000055555558660f in read_root_dse (pb=0x5555559db910, e=0x555555aab500, entryAfter=0x0,

returncode=0x7fffffff72a4, returntext=0x7fffffff72e0 "", arg=0x0)
at ldap/servers/slapd/rootdse.c:196

4 0x00007ffff7ae1691 in dse_call_callback (pdse=0x555555927a00, pb=0x5555559db910, operation=4,

flags=1, entryBefore=0x555555aab500, entryAfter=0x0, returncode=0x7fffffff72a4,
returntext=0x7fffffff72e0 "") at ldap/servers/slapd/dse.c:2634

5 0x00007ffff7adf256 in dse_search (pb=0x5555559db910) at ldap/servers/slapd/dse.c:1737

6 0x00007ffff7b27e91 in op_shared_search (pb=0x5555559db910, send_result=1)

at ldap/servers/slapd/opshared.c:802

7 0x00007ffff7b3deb4 in search_internal_callback_pb (pb=0x5555559db910,

callback_data=0x7fffffffda10, prc=0x7ffff7b3d939 <internal_plugin_result_callback>,
psec=0x7ffff7b3d84c <internal_plugin_search_entry_callback>,
prec=0x7ffff7b3d8cc <internal_plugin_search_referral_callback>)
at ldap/servers/slapd/plugin_internal_op.c:783

8 0x00007ffff7b3da0b in search_internal_pb (pb=0x5555559db910)

at ldap/servers/slapd/plugin_internal_op.c:636

9 0x00007ffff7b3d656 in slapi_search_internal (base=0x7fffea672a95 "", scope=0,

filter=0x7fffea672a85 "(objectclass=*)", controls=0x0, attrs=0x7fffffffdaf0, attrsonly=0)
at ldap/servers/slapd/plugin_internal_op.c:480

10 0x00007fffea671693 in views_cache_build_view_list (pViews=0x7fffea874040 )

at ldap/servers/plugins/views/views.c:1154

11 0x00007fffea670941 in views_cache_create () at ldap/servers/plugins/views/views.c:448

12 0x00007fffea670304 in views_start (pb=0x555555ac09f8) at ldap/servers/plugins/views/views.c:217

13 0x00007ffff7b36ffe in plugin_call_func (list=0x5555559d20f0, operation=212, pb=0x555555ac09f8,

call_one=1) at ldap/servers/slapd/plugin.c:1987

14 0x00007ffff7b36e7d in plugin_call_one (list=0x5555559d20f0, operation=212, pb=0x555555ac09f8)

at ldap/servers/slapd/plugin.c:1937

15 0x00007ffff7b3673f in plugin_dependency_startall (argc=7, argv=0x7fffffffe438,

errmsg=0x7ffff7b9ce9c "plugin startup failed\n", operation=212, plugin_list=0x0)
at ldap/servers/slapd/plugin.c:1745

16 0x00007ffff7b36d94 in plugin_startall (argc=7, argv=0x7fffffffe438, plugin_list=0x0)

at ldap/servers/slapd/plugin.c:1898

17 0x000055555557d686 in main (argc=7, argv=0x7fffffffe438) at ldap/servers/slapd/main.c:1059

}}}

attachment
0001-Ticket-48363-Support-for-rfc3673-to-return-operation.2.patch

Thanks for the review Noriko.

I didn't have that issue with building, but you did correctly notice I was missing that header. I have added it now. I've also fixed up the other points you raised. Here is a diff of what I changed, and the rebased patch is attached to the thread.

{{{

diff --git a/ldap/servers/slapd/features.c b/ldap/servers/slapd/features.c
index aade6c3..077002e 100644
--- a/ldap/servers/slapd/features.c
+++ b/ldap/servers/slapd/features.c
@@ -1,6 +1,5 @@
/** BEGIN COPYRIGHT BLOCK
- * Copyright (C) 2001 Sun Microsystems, Inc. Used by permission.
- * Copyright (C) 2005 Red Hat, Inc.
+ * Copyright (C) 2016 Red Hat, Inc.
* All rights reserved.
*
* License: GPL (version 3 or any later version).
@@ -15,6 +14,7 @@

#include
#include "slap.h"
+#include "slapi-plugin.h"

static char *supported_features = NULL;
static Slapi_RWLock supported_features_lock = NULL;
@@ -28,7 +28,7 @@ init_features( void )
"init_features: failed to create lock.\n");
exit(1);
}
- slapi_register_supported_feature( LDAP_FEATURE_RFC3673_ALLOPERATIONAL );
+ slapi_register_supported_feature( LDAP_FEATURE_ALL_OP_ATTRS );
}

int
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 5361973..f66ccd0 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -5619,7 +5619,9 @@ char *slapi_get_supported_extended_ops_copy( void );
* routines for dealing with supported features
/

-#define LDAP_FEATURE_RFC3673_ALLOPERATIONAL "1.3.6.1.4.1.4203.1.5.1"
+#ifndef USE_OPENLDAP
+#define LDAP_FEATURE_ALL_OP_ATTRS "1.3.6.1.4.1.4203.1.5.1"
+#endif

int slapi_get_supported_features_copy( char ***ftroidsp );

}}}

Looks good.

It's a very minor thing, but this is more straightforward?
{{{
+#ifndef LDAP_FEATURE_ALL_OP_ATTRS
+#define LDAP_FEATURE_ALL_OP_ATTRS "1.3.6.1.4.1.4203.1.5.1"
+#endif
}}}
For instance, this header "slapi-plugin.h" has lots of similar macros...
{{{
/
* filter types
* openldap defines these, but not mozldap
/

ifndef LDAP_FILTER_AND

define LDAP_FILTER_AND 0xa0L

endif

}}}

commit f4b97595525e07ca22b1e361edabf207d0dbba48

To ssh://git.fedorahosted.org/git/389/ds.git
1d4fcea..08c40e8 master -> master

Thanks!

git patch file (master) -- one line fix; symptom: setup ds fails with server's crash.
0001-Ticket-48363-Support-for-rfc3673-to-return-operation.3.patch

One line fix; pushed to master:
4e39b5b..0cf249e master -> master
commit 0cf249e9e497caa05f3abc5f4e8090c1df91ccab

attachment
0001-Ticket-48363-CI-test-add-test-suite.patch

Yep, the test passes for me.

I think that it should be named something like "dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py"

I also liked the comments in my test that referenced the rfc. Maybe you could transplant these?

Otherwise, functionality wise, I'm happy to ack, and if you want to make these other changes, that would be awesome.

William, thank you for the review. :) Applied.

To ssh://git.fedorahosted.org/git/389/ds.git
17f30c6..03c1e86 master -> master
commit 03c1e860a58c07b6c24767178aa3048ea7771c30
Author: Simon Pichugin spichugi@redhat.com
Date: Fri May 20 10:54:51 2016 +0200

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1694

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.