Re: [PATCH] Completed TODO: svn_stream_copy3()

Edmund Wong wrote on Fri, 10 Apr 2009 at 23:34 +0800:
> >
> > if (cancel_func)
> > {
> > err = cancel_func(cancel_baton);
> > if (err)
> > break;
> > }
>
> Is this allowed?
>
> if (err = cancel_func(cancel_baton))
> break;
>

It's wrong, because cancel_func may be NULL.

If I were you, I'd try to keep the logic changes minimal --- i.e., avoid
reordering the various checks on LEN and CANCEL_FUNC. They work well as
they are, so no reason to change them. (If you change something, you have
to think whether the change is correct...)

As a bonus, it'll make the diff easier to read/understand.

Daniel

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1646177
Received on 2009-04-11 11:47:35 CEST