On Sat, Apr 28, 2018 at 5:59 AM, Branko Čibej <br...@apache.org> wrote:
> On 27.04.2018 21:23, Greg Stein wrote:
> > At one point, we used to use an svn_array_find() or some such. We
> > deprecated it. Creating a function to do comparisons, then set up a
> > function call... It was just annoying. Iteration over an APR array is
> > easy, and very clear. There is basically no code or mental overload.
> > Using a comparator function, there is.
>
> static int compare(const void* x, const void* y) {
> return strcmp(*(const char**) x, (const char*) y));
> // Oops ... is the first argument the pointer to the
> // array element and the second the search key, or is
> // it the other way around? Or do we have to dereference
> // the search key in order to make these arguments
> // symmetric?
> }
>
> // ... lots of code in between
>
> int i = apr_array_find(array, compare, "key");
>
> The first form is definitely easier to understand and nicely localized.
> "Explicit is better than implicit" and all that.
>
If you wanted to pursue this, I'd definitely apr_{type}_compare_set()
the comparator callback at array creation. Saves stack prep on lots
of calls, and avoids individual errors passing a comparator off in
some infrequently used code path. Use apr_uintptr_t if you want to
portably pass an int-or-ptr portably as the lhs/rhs argument. But
Greg makes a good case for comparing structs passed on the stack
(svn_errno is such a struct), and this requires the _compare_set()
to accept an incomplete type. This likely can't be done in 1.x (and
would be problematic when porting certain code to 2.x) because
apr_array_t was a very transparent type (where to hide the pointer
to comparator fn?)
