Don't say it @mreynolds, I just spotted this incorrect comment syntax.

Pre-empting "why did you touch so much in libglobs.c". It's because of this call here, where we add the capability for a initval generator function to each slot type for the config.

Please don't use java style comments

AHhhhhh I swear I did a search for them all in the C code (Rust uses // for comments and /// for docs so we'll have to deal with it there).

@mreynolds and @mhonek ping :)

Review reminder @mreynolds

Looks good, but I want to run a few tests with the patch. Question are there platforms that won't have rust (do we really need the #ifdefs)?

The ifdef is more about "readiness" and when both SUSE/RedHat say "yes we can ship this to production", so it gives us a fast on/off switch. Perfect world, we shouldn't need it, but "just in case". This way we can do a release, if it goes wrong we can ifdef it off rather than reverting the patch.

Is that a reasonable comment?

@firstyear, Matus has some comments on your design doc about adding a token refresh option:

https://github.com/marcus2376/389wiki/pull/17

Just bringing it to your attention

doe should "does"

Compiler error:

../389-ds-base/ldap/servers/slapd/bind.c: In function ‘do_bind’:
../389-ds-base/ldap/servers/slapd/bind.c:754:26: warning: implicit declaration of function ‘pw_verify_token_dn’; did you mean ‘pw_verify_be_dn’? [-Wimplicit-function-declaration]
754 | rc = pw_verify_token_dn(pb);
| ^~~~~~~~~~~~~~~~~~
| pw_verify_be_dn

Probably need to add it to header somewhere

Thanks for the comments @mreynolds I'll have a look at this today. I'm surprised about the missing definition for the header though :(

Maybe I should change my compiler opts to -Werror?

Right, updated based on your feedback. Sometimes I wonder if you missed a career in spell checking :) :)

Also noted an issue with linking so that's resolved now.

Any other comments @mreynolds or @mhonek?

Your patch no longer cleanly applies to Master branch, but it looks like you still have the same problem I reported in https://pagure.io/389-ds-base/pull-request/50703#comment-104376

in bind.c you need to include pw_verify.h

Your patch no longer cleanly applies to Master branch, but it looks like you still have the same problem I reported in https://pagure.io/389-ds-base/pull-request/50703#comment-104376
in bind.c you need to include pw_verify.h

Blah, every time I click "Submit Comment" I think "Wait! Double check ______". Yes bind.c had this include already. I would really like a patch that applies cleanly to give my full ACK, so get this patch rebased and I'll give it a quick once over and we can finally merge it!

@mreynolds Doing it right now :)

@mreynolds Doing it right now :)

Thanks I'll review this tomorrow!

ACK! Nice. I ran a few tests everything seems okay. Lets get this in, and if anything pops up we'll fix it.

Thanks so much @mreynolds!

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 pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3758

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.