Re: [WIP PATCH 1/7] Add skeleton RA svnclient

From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Jun 25, 2010, 11:30 UTC
Message-ID: <alpine.561.2.00.1006251412170.2144@daniel2.local>
In-Reply-To: <AANLkTimirObq-HBrB4rQdAprN8mfd92rfFKSegJKwMsc@mail.gmail.com>

Ramkumar Ramachandra wrote on Fri, 25 Jun 2010 at 14:07 -0000:

Show 21 quoted lines

> Daniel Shahaf wrote:
> > Jonathan Nieder wrote on Fri, 25 Jun 2010 at 03:14 -0000:
> >> What is a baton?
> >>
> >
> > The context object for a callback.
> >
> > You call:
> >
> >    some_function(your_callback_function, your_baton)
> >
> > which then calls:
> >
> >    your_callback_function(your_baton, other_arguments)
> 
> In general, I've found that batons are void * objects in which you can
> stuff anything you like and pass around from function to function:
> I've abused them quite heavily in my code by stuffing all kinds of
> things into them: see fb->eb->* in my current code for an example of
> this.
>

We do that too sometimes:

% pwd $trunk_wc/subversion/libsvn_client/ % grep -- "->eb->" *.c | head commit_util.c: return (*db->eb->real_editor->add_file)(path, db->real_baton, commit_util.c: return (*db->eb->real_editor->delete_entry)(path, revision, commit_util.c: return (*db->eb->real_editor->open_file)(path, db->real_baton, commit_util.c: return (*fb->eb->real_editor->close_file)(fb->real_baton, commit_util.c: return (*fb->eb->real_editor->change_file_prop)(fb->real_baton, commit_util.c: return (*fb->eb->real_editor->apply_textdelta)(fb->real_baton, ...

Show 23 quoted lines

> >> [...]
> >> > +  void *wrapped_edit_baton;
> >> [...]
> >> > +  void *edit_baton;
> >> > +  void *wrapped_dir_baton;
> >> [...]
> >> > +  void *edit_baton;
> >> > +  void *wrapped_file_baton;
> >>
> >> Are these opaque types necessary?
> >>
> >
> > The convention in Subversion's code is to convert the void * to
> > a concrete_baton_t * only inside the callback.  If you wish to declare
> > these, e.g., as
> >
> >    debug_editor_baton_t *wrapped_baton;
> >
> > You can probably do that too.
> 
> The function prototypes in libsvn contain void * parameters
> corresponding to batons, so I'd have to typecast explicitly to avoid
> any warnings.

By the way, a common pitfall is to mis-treat the void *baton argument as the wrong type of baton. That is, code of the form

    f(void *baton) {
        foo_baton_t *fb = baton;
    }

instead of

    f(void *baton) {
        bar_baton_t *bb = baton;
    }

Show 5 quoted lines

> I think Jonathan's also referring to the absence of the
> "struct" keyword everywhere, as that is against Git policy.
> Unfortunately, everything is typedef'ed in libsvn, and we cannot do
> much about that.
>

Usually we do

    typedef struct svn_error_t svn_error_t;

so you can add the 'struct' back if you want.

Show 20 quoted lines

> >> What does this do?  Is SVN_ERR for debugging?
> >
> > That's how we implement exception throwing in C.  SVN_ERR means "if this
> > returned a non-NULL svn_error_t *, then return that error to our
> > caller".
> >
> > The other pattern does
> >
> >    svn_error_t *err = svn_stream_printf();
> >
> > and then inspects err and err->apr_err to decide whether to ignore the
> > error or return it (possibly wrapped).
> 
> >> Where does the output go?
> >>
> >
> > SVN_ERR does not print anything.  It may return(), though.
> 
> Embarrassingly enough, write_indent does exactly what it says it does:
> It writes some spaces (or indent) to eb->out

To represent nesting.

> Note however, that even with all this error handling, the most common
> type of error I get by far is the segfault: I'll make an effort to
> document the pitfalls.
>

Note there are several kinds of errors that can cause a segfault. (@Ram, you've encountered some of them already, I know.)

Show 11 quoted lines

> >> I take it these are callbacks?  Is there overview documentation for
> >> them somewhere?
> >>
> >
> > svn_delta_editor_t in
> > http://svn.apache.org/repos/asf/subversion/trunk/subversion/include/svn_delta.h
> 
> Yes, they are callbacks that are fired automatically by the editor. In
> svn_delta.h, look at struct svn_delta_editor_t (and the corresponding
> doxygen-style comments).
>

The compiled doxygen docs are available on <http://subversion.apache.org/docs/#api>. Personally I always just read the header file directly (with :set filetype=c.doxygen).