Daniel Shahaf <[email protected]> wrote:
>C. Michael Pilato wrote on Fri, Jun 21, 2013 at 09:00:46 -0400:
>> On 06/21/2013 06:43 AM, Daniel Shahaf wrote:
>> > I still think the logic better belongs inside
>svn_repos__validate_prop().
>> > (Not the least because it has three other callers which also need
>to
>> > accept NULL values.)
>> >
>> > --- subversion/libsvn_repos/fs-wrap.c (revision 1495373)
>> > +++ subversion/libsvn_repos/fs-wrap.c (working copy)
>> > @@ -172,6 +172,10 @@ svn_repos__validate_prop(const char *name,
>> > {
>> > svn_prop_kind_t kind = svn_property_kind2(name);
>> >
>> > + /* No property is mandatory. */
>> > + if (value == NULL)
>> > + return SVN_NO_ERROR;
>> > +
>> > /* Disallow setting non-regular properties. */
>> > if (kind != svn_prop_regular_kind)
>> > return svn_error_createf
>> >
>>
>> +1. This is precisely what I was suggesting as well.
>
>Committed and nominated for backport. Arwin, if you think that's not
>a good solution, we can still amend it in subsequent revisions. Thanks
>for the patch.
>
>Daniel
No. I get it now. It's better to check if it's a delete and easy out from
within the validation code so all callers of the function will benefit. They
don't have to perform the check themselves.
Regards,
Arwin Arni