please use slapi_ch_free_string()

slapi_ch_free_string() please

and slapi_ch_free_string() as well :-)

If this wasn't run through a ASAN build could you please do so? Thanks!

Rather than extensibleObject, can we add a proper configuration class type into 01core389.ldif please?

Can you explain this more please? Is this when you try to search based on a re-written attribute? Shouldn't that mean the rewritten attr is in schema, even if it's generated?

Copyright year?

If it's not used now, shouldn't we either remove it, or use it ;)

Small detail, you can do for(size_t i = 0; ...) since we use C99 now :D

Interesting idea, but given that rewriters will all need to "search" through the filter, couldn't we write this function as "rewrite_objectcategory", and the framework does the filter iteration, searching etc, and then just applies rewrite_objectcategory to the needed elements? That would reduce every rewriter having to know how do to iteration, and the plugin just becomes an expression of what single filter element we want to update.

This smeems pretty small as a header, couldn't we just inline this in adfilter.c?

How is this user_%d working?

@mreynolds , @firstyear , thanks for your review.
I agree with all you comments and will update the PR. I will start with ASAN check and resend a PR. I like the idea of making the filter parsing generic and will try to implement it (possibly in https://pagure.io/389-ds-base/pull-request/50981)

@tbordaz I appreciate it, I look forward to reviewing the update!

@mreynolds @firstyear , finally I was able to rework the patch :)

indentation is off :-)

indentation is off here too

Shouldn't format be freed some where? Or is it consumed?

I would feel better if this went through an ASAN test to make sure we aren't leaking anything. Thanks!

Thanks @mreynolds for the review. Surprisingly ASAN did not report a leak but you were right 'format' was leaking. Patch fixed

Thanks @mreynolds for the review. Surprisingly ASAN did not report a leak but you were right 'format' was leaking. Patch fixed

So does that means the CI test is not exercising all the code? I saw other suspicious potential leaks as well, and that's why I asked about the ASAN build. I'm going to point out some of those areas I was concerned about next...

What frees the berval & newval?

What frees the berval & newval?

This was my only other concern, so it's OK then you get my ack

The berval (bval) is a pointer into the Slapi_filter structure of the operation. The Slapi_filter is freed at the end of the operation.
The newval, replace the current bval->bv_val and so is also freed by the operation.
The replaced bv->bv_val is freed by the substitute_shortcut callback during the replacement.

Thanks, ack!

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/4041

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.