On Fri, Apr 10, 2009 at 17:34, Edmund Wong <ed_at_kdtc.net> wrote:
>...
>> You cancel_func == NULL, then you can avoid the second if, since there
>> is no way err could get set:
>
> In hindsight, I can simply say this is one of those
> "what the smeg was I thinking" moments. It's so
> obvious that I wasn't really thinking clearly. Sorry.
No worries, and don't be so hard on yourself.
>...
>> Skip this else branch, and just do the len test here.
>
> I wasn't sure if at len== 0, it would break out of
> the loop. Now I think of it, if len == 0, wouldn't
> the stream_read() enter an error state?
Hard to know. We have a lot of different stream implementations. I
don't recall any stream read functions that look for len==0, so they
probably won't error.
>>> - return svn_error_compose_create(svn_stream_close(from),
>>> - svn_stream_close(to));
>>> + return svn_error_compose_create(err,err2);
>>
>> If err==NULL, then it will return whatever err2 is... in this case,
>> any potential problem with closing the streams.
>>
>> Style note: there should be a space after that comma.
>
> Yes. Stefan told me that. I still haven't gotten that
> into my thick skull of mine.
>
> Thanks! I'll get on to the patch asap.
The last one you posted looks good, minus the extra parens. I'll go
ahead and apply it. Thanks!
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1672916
Received on 2009-04-12 19:55:12 CEST