On Wed, Jan 26, 2011 at 2:46 PM, Hyrum K Wright <hy...@hyrumwright.org> wrote: > On Tue, Jan 25, 2011 at 11:41 PM, Daniel Shahaf <d...@daniel.shahaf.name> > wrote: >> Johan Corveleyn wrote on Wed, Jan 26, 2011 at 03:31:11 +0100: >>> Revving svn_diff_fns_t: what do you mean with parallelizing it? I must >>> admit that I don't really know (yet) how to go about that. Very early >>> during the branch work, danielsh pointed out that I modified this >>> public struct (vtable for reading data from datasources), so it should >>> be revved. Is it listed/documented somewhere what should be done for >>> that (community guide perhaps)? >> >> Briefly, revving a function includes re-declaring it, updating the old >> declaration to be a diff against the new one, marking it as deprecated >> (using doxygen and C preprocessor designators), and re-implementing the >> old function as a deprecated.c wrapper around the new one. >> >> For a struct, you need to re-declare the struct and revv functions that >> use it. Also, don't forget to add a constructor function >> (svn_foo_t_create(), but s/_t_/_/) (and possibly a duplicator function), >> and forbid people from defining variables of the struct type directly. >> >> I'm not sure HACKING contains this. On the other hand, the public >> header files contain plenty of examples of everything I just said... > > For testing purposes, I usually find it easiest to work "from the > bottom up". In other words, rev the lowest API in the call stack, > write the appropriate wrapper for the existing API, and test and > commit. This ensures that the existing API (now implemented as a > wrapper) still behaves the same way, but it doesn't yet exercise > whatever the new functionality is. I then just work my way up the > stack for each related function. Structs are a little more involved, > but can use similar principles. > > Johan, if you don't mind, I can put some of my (admittedly limited) > tuits into looking at what needs to be done to rev the above struct.
I would definitely, absolutely not mind at all :-). Quite the contrary. As a summary of what I did with svn_diff_fns_t (all in the context of subversion/libsvn_diff): - Added new function 'datasources_open' to svn_diff_fns_t (maybe better to be named 'datasources_open_all' or something to have more than 1 character difference with the other existing function 'datasource_open' :-), but I said that already). - This new function is implemented in diff_file.c (the 'real' work of prefix/suffix scanning) and in diff_memory.c (dummy implementation - doesn't do anything for prefix/suffix scanning). Those were the only internal implementors/providers of that vtable. - I sent a mail once to the dev-list, asking if there were other known implementors [1], but got no response. Of course, that's not a definitive survey :-). - The new vtable->datasources_open function is called from diff.c, diff3.c and diff4.c. - The only internal caller of the "old" function 'datasource_open' (for a single datasource) doesn't call it anymore (token.c#svn_diff__get_tokens) (there is no need anymore, since the callers in diff.c, diff3.c and diff4.c already open the datasources with 'datasources_open' before calling svn_diff__get_tokens). Cheers, -- Johan [1] http://svn.haxx.se/dev/archive-2010-12/0083.shtml
