Ivan Zhakov wrote on Wed, Jun 25, 2014 at 20:03:11 +0400:
> On 25 June 2014 19:54, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> wrote:
> > On Wed, Jun 25, 2014 at 5:46 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> >>
> >> On 25 June 2014 19:24, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
> >> wrote:
> >> > On Wed, Jun 25, 2014 at 4:57 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> >> >>
> >> >> On 21 June 2014 17:15, <stefan2_at_apache.org> wrote:
> >> >> > Author: stefan2
> >> >> > Date: Sat Jun 21 15:15:30 2014
> >> >> > New Revision: 1604421
> >> >> >
> >> >> > URL: http://svn.apache.org/r1604421
> >> >> > Log:
> >> >> > Append index data to the rev data file instead of using separate
> >> >> > files.
> >> >> >
> >> >> > This gives unpacked FSFS format 7 similar I/O characteristics and
> >> >> > disk
> >> >> > space usage as earlier formats. It also eliminates the need for
> >> >> > retries
> >> >> > after a potential background pack run because each file is now always
> >> >> > consistent with itself (earlier, data or index files might already
> >> >> > have
> >> >> > been deleted while the respective other was still valid). Overall,
> >> >> > most of this patch is removing code necessary to handle separate
> >> >> > files.
> >> >> >
> >> >> > The new file format is simple:
> >> >> >
> >> >> > <rep data><l2p index><p2l index><footer><footer length>
> >> >> >
> >> >> > with the first three being identical what we had before. <footer
> >> >> > length>
> >> >> > is a single byte giving the length of the preceding footer, so it's
> >> >> > easier to extract than the pre-f7 rev trailer and there is only one
> >> >> > per pack / rev file.
> >> >> >
> >> >> [..]
> >> >>
> >> >>
> >> >> >Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> >> >> >URL:
> >> >> >
> >> >> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1604421&r1=1604420&r2=1604421&view=diff
> >> >>
> >> >> >
> >> >> > > >==============================================================================
> >> >> >--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> >> >> >+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Jun 21
> >> >> > 15:15:30
> >> >> > 2014
> >> >> >@@ -1449,15 +1449,35 @@ write_revision_zero(svn_fs_t *fs)
> >> >> >+ SVN_ERR(svn_io_file_create_binary(path_revision_zero,
> >> >> >+ "PLAIN\nEND\nENDREP\n"
> >> >> >+ "id: 0.0.r0/2\n"
> >> >> >+ "type: dir\n"
> >> >> >+ "count: 0\n"
> >> >> >+ "text: 0 3 4 4 "
> >> >> >+ "2d2977d1c96f487abe4a1e202dd03b4e\n"
> >> >> >+ "cpath: /\n"
> >> >> >+ "\n\n"
> >> >> >+
> >> >> >+ /* L2P index */
> >> >> >+ "\0\x80\x40" /* rev 0, 8k entries per page
> >> >> > */
> >> >> >+ "\1\1\1" /* 1 rev, 1 page, 1 page in
> >> >> > 1st
> >> >> > rev */
> >> >> >+ "\6\4" /* page size: bytes, count */
> >> >> >+ "\0\xd6\1\xb1\1\x21" /* phys offsets + 1 */
> >> >> >+
> >> >> >+ /* P2L index */
> >> >> >+ "\0\x6b" /* start rev, rev file size
> >> >> > */
> >> >> >+ "\x80\x80\4\1\x1D" /* 64k pages, 1 page using 29
> >> >> > bytes */
> >> >> >+ "\0" /* offset entry 0 page 1 */
> >> >> >+ /* len, item & type, rev,
> >> >> > checksum */
> >> >> >+ "\x11\x34\0\xf5\xd6\x8c\x81\x06"
> >> >> >+ "\x59\x09\0\xc8\xfc\xf6\x81\x04"
> >> >> >+ "\1\x0d\0\x9d\x9e\xa9\x94\x0f"
> >> >> >+ "\x95\xff\3\x1b\0\0" /* last entry fills up 64k
> >> >> > page
> >> >> > */
> >> >> >+
> >> >> >+ /* Footer */
> >> >> >+ "107 121\7",
> >> >> >+ 107 + 14 + 38 + 7 + 1, fs->pool));
> >> >> This constant makes code unreadable, unmaintainable and very error
> >> >> prone.
...
> I saw it, but it doesn't make code easier to maintain.
Ivan, that's the third time in as many emails that you say "the new code
is hard to maintain". Could you please explain *how* you find it hard
to maintain?
I assume you dislike the magic numbers and would prefer to see sizeof()
uesd instead, e.g.,
#define REV0_FILE_PART_1 "foo"
#define REV0_FILE_PART_2 "bar"
svn_file_create_binary(REV0_FILE_PART_1 REV0_FILE_PART_2,
sizeof(REV0_FILE_PART_1)-1
+ sizeof(REV0_FILE_PART_2)-1);
Anyway, that's just me guessing. Could you please clarify what exactly
makes the code unmaintainable in your opinion?
Thanks.
Received on 2014-06-25 20:12:18 CEST