On 11.11.2016 15:30, Branko Čibej wrote:
On 11.11.2016 15:04, Patrick Steinhardt wrote:
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).
I agree; we have other instances of such behaviour, e.g., for returned
revision properties in svn_client_log5() where NULL array means return
all propertes and empty array means return none.
Since svn_client_list4() is new on trunk, I suggest you just send in a
patch that fixes this behaviour as you described.
I think having NULL and an empty list describing the
same thing is not a good idea. So, if we allow NULL,
then the empty list should become the degenerate
case of "nothing could possibly match".
Since API users say they prefer NULL over an empty
array, I changed the code accordingly in r1769485.
-- Stefan^2.
