Evgeny Kotkov wrote on Thu, 31 Aug 2017 19:05 +0300:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> > Is adding this function an ABI-compatible change? The docstring of
> > svn_delta_editor_t does say """
> >
> > * @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.
> >
> > """, but an API consumer might have interpreted this note as meaning "You may
> > use malloc(..., sizeof(svn_delta_editor_t)) if you take care to initialize
> > all struct members", in which case, his code will not be ABI compatible
> > with 1.10.
>
> I think that adding this callback does not affect the ABI compatibility.
> The note says "Don't try to allocate one of these yourself", whereas the
> malloc(..., sizeof(svn_delta_editor_t)) example does exactly the opposite.
I'm not sure I'm getting through here.
The note does say "Don't allocate one of these yourself" but in the
second sentence implies that the reason for this is that if you allocate
it yourself and don't initialize all pointer-to-function members,
Trouble will ensue. Therefore, the note could be construed as meaning
that it is okay to allocate one of these yourself if you take care to
initialize all pointer members.
In other words: the comment could have been interpreted as a piece
of advice --- "it is more robust to use _default_editor() than to allocate
with malloc" --- as opposed to a blanket prohibition on allocating this
struct type with malloc.
(If one uses malloc and doesn't initialize all pointers, the result
would be that an uninitialized pointer-to-function struct member is
dereferenced.)
In contrast, most of our other structs explicitly say that "Don't
allocate yourself because we may add new fields in the future". This
struct does not give that reason.
Makes sense?
I suppose can just add an API erratum and/or release notes entry about this.
Thanks,
Daniel
(I'll reply to your other points separately)
Received on 2017-09-01 05:07:23 CEST