Re: [PATCH] svn_client_list4: accept `NULL` patterns

On Wed, Nov 09, 2016 at 02:05:38PM +0100, Stefan Sperling wrote:
> On Wed, Nov 09, 2016 at 11:27:47AM +0100, Patrick Steinhardt wrote:
> > Hi,
> >
> > while continuing my work on the obstructed checkout patch I just
> > updated to the new `svn_client_list4` call, which introduced a
> > new `patterns` parameter. I initially run into a `NULL` pointer
> > derefeernce, as I expected that the patterns parameter may also
> > be `NULL`, as it is often the case for parameters which are not
> > in use.
> >
> > The attached patch changes the function to accept a `NULL`
> > argument for `parameters` in addition to an empty array, which is
> > mostly a convenience/consistency thing for callers of the new
> > function.
> >
> > Regards
> > Patrick
>
> I agree with this semantic API change.
>
> Should this patch not also adjust the docstring for svn_client_list4()
> in the file subversion/include/svn_client.h?

Probably, yes. Thinking about this some more, I'm not that happy
with this patch. I still think it should be possible for callers
to pass `NULL`, meaning that no filtering should be done. But I'm
not particularly happy about an empty array meaning that no
filtering should be done, as well.

A use case is that first, the array is filled dynamically with
items to filter by. But in case that the array is not filled with
anything, I'd expect the function to return nothing instead of
everything. So maybe the behavior should even be:

- NULL pointer: no filtering at all
- empty array: return nothing
- filled array: return only items matching at least one of the
  items

For me as a developer, this behavior would match my expectations
much better (even though one could argue that one should simply
read the docstring).

Regards
Patrick

-- 
Patrick Steinhardt, Entwickler
elego Software Solutions GmbH, http://www.elego.de
Gebäude 12 (BIG), Gustav-Meyer-Allee 25, 13355 Berlin, Germany
Sitz der Gesellschaft: Berlin, USt-IdNr.: DE 163214194
Handelsregister: Amtsgericht Charlottenburg HRB 77719
Geschäftsführer: Olaf Wagner