Evgeny Kotkov wrote on Thu, 31 Aug 2017 19:05 +0300: > Daniel Shahaf <d...@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)