Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

Philip Martin wrote:
> Evgeny Kotkov writes:
>> I sketched this option in the attached patch. With this patch applied, we
>> could rework the r1680819 fix like below. What do you think?
>
> Yes, that looks good.

Just a couple of comments, from a quick read of the patch.

+/** Flushes all available internal buffers in a generic @a stream. If
+ * @sync is non-zero, it causes any buffered data to be written to the
+ * underlying device (for example, to the disk for a file stream).

I'm not sure about having the 'sync' flag. What does that mean for a
non-disk stream? Does the API really need to support two different
ways of flushing a generic stream? I feel probably not. If we need to
support two different ways of flushing a disk stream, could we achieve
that in a better way? Perhaps the code that opens the disk stream
should specify this option when opening the disk stream, rather than a
generic writer specifying this option when it calls 'flush'?

In flush_handler_lazyopen, I think the code should avoid opening the
stream first unless the 'always open' mode is active: see
close_handler_lazyopen().

- Julian
Received on 2015-05-26 16:56:07 CEST