[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1803143 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_ra_serf/ mod_dav_svn/

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Tue, 5 Sep 2017 20:21:04 +0300

Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:

>> Also, I am not against the idea of tweaking the note by saying something
>> like "...because we may add new fields in the future", but I don't think
>> that it is required either. (In other words, I am +0 to that.)
>
> Done in r1807028.

I think that although r1807028 provides the additional information to the
users, it simultaneously makes the API requirements less strict:

> - * @note Don't try to allocate one of these yourself. Instead, always
> - * use svn_delta_default_editor() or some other constructor, to ensure
> - * that unused slots are filled in with no-op functions.
> + * @note Fields may be added to the end of this structure in future
> + * versions. Therefore, users shouldn't allocate structures of this
> + * type, to preserve binary compatibility.
> + *
> + * @note It is recommended to use svn_delta_default_editor() or some other
> + * constructor, to ensure that unused slots are filled in with no-op functions.

While it does clarify that new fields can be added to the struct, this
changeset also replaces words like "don't try" and "always" with "shouldn't"
and "is recommended" thus making the requirements a recommendation:

 - "Don't try to allocate one of these yourself" became "users shouldn't
   allocate structures"

 - "Instead, always use svn_delta_default_editor()" is now "It is
   recommended to use svn_delta_default_editor()"

Perhaps, a better way would be to keep the original wording, but mention
that the structure may be extended, as in this alternative patch:

[[[
--- subversion/include/svn_delta.h (revision 1806549)
+++ subversion/include/svn_delta.h (working copy)
@@ -694,8 +694,10 @@ svn_txdelta_skip_svndiff_window(apr_file_t *file,
  * as it produces the delta.
  *
  * @note Don't try to allocate one of these yourself. Instead, always
- * use svn_delta_default_editor() or some other constructor, to ensure
- * that unused slots are filled in with no-op functions.
+ * use svn_delta_default_editor() or some other constructor, to avoid
+ * backwards compatibility problems if the structure is extended in
+ * future releases and to ensure that unused slots are filled in with
+ * no-op functions.
  *
  * <h3>Function Usage</h3>
  *
]]]

Regards,
Evgeny Kotkov
Received on 2017-09-05 19:21:39 CEST

This is an archived mail posted to the Subversion Dev mailing list.