svn commit: r348285 - projects/fuse2/tests/sys/fs/fusefs

[Enji Cooper]

> On May 25, 2019, at 20:52, Alan Somers <asomers at freebsd.org> wrote:
> 
> Author: asomers
> Date: Sun May 26 03:52:35 2019
> New Revision: 348285
> URL: https://svnweb.freebsd.org/changeset/base/348285
> 
> Log:
>  fusefs: more build fixes
> 
>  * Fix printf format strings on 32-bit OSes
>  * Fix -Wclass-memaccess violation on GCC-8 caused by using memset on an object
>    of non-trivial type.
>  * Fix memory leak in MockFS::init
>  * Fix -Wcast-align error on i386 in expect_readdir
>  * Fix some heterogenous comparison errors on 32-bit OSes.
> 
>  Sponsored by:    The FreeBSD Foundation
> 
> Modified:
>  projects/fuse2/tests/sys/fs/fusefs/mockfs.cc
>  projects/fuse2/tests/sys/fs/fusefs/mockfs.hh
>  projects/fuse2/tests/sys/fs/fusefs/read.cc
>  projects/fuse2/tests/sys/fs/fusefs/setattr.cc
>  projects/fuse2/tests/sys/fs/fusefs/utils.cc
> 
> Modified: projects/fuse2/tests/sys/fs/fusefs/mockfs.cc
> ==============================================================================
> --- projects/fuse2/tests/sys/fs/fusefs/mockfs.cc    Sat May 25 23:58:09 2019    (r348284)
> +++ projects/fuse2/tests/sys/fs/fusefs/mockfs.cc    Sun May 26 03:52:35 2019    (r348285)

...

> -    in = (mockfs_buf_in*) malloc(sizeof(*in));
> +    in = new mockfs_buf_in;
>    ASSERT_TRUE(in != NULL);
> -    out = (mockfs_buf_out*) malloc(sizeof(*out));
> +    out = new mockfs_buf_out;
>    ASSERT_TRUE(out != NULL);
> 
>    read_request(in);
>    ASSERT_EQ(FUSE_INIT, in->header.opcode);
> 
> -    memset(out, 0, sizeof(*out));
>    out->header.unique = in->header.unique;
>    out->header.error = 0;
>    out->body.init.major = FUSE_KERNEL_VERSION;
> @@ -418,7 +423,8 @@ void MockFS::init(uint32_t flags) {
>    SET_OUT_HEADER_LEN(out, init);
>    write(m_fuse_fd, out, out->header.len);
> 
> -    free(in);
> +    delete out;
> +    delete in;

Given that the tests are currently using C++14, it seems like it would make sense to use smart pointers here to manage the scope/lifetime of the objects, as this could leak if someone added an early return or an exception was thrown. Ref: https://en.cppreference.com/book/intro/smart_pointers.

> }
> 
> void MockFS::kill_daemon() {
> 
> Modified: projects/fuse2/tests/sys/fs/fusefs/mockfs.hh
> ==============================================================================
> --- projects/fuse2/tests/sys/fs/fusefs/mockfs.hh    Sat May 25 23:58:09 2019    (r348284)
> +++ projects/fuse2/tests/sys/fs/fusefs/mockfs.hh    Sun May 26 03:52:35 2019    (r348285)
> @@ -166,7 +166,7 @@ union fuse_payloads_out {
>    fuse_create_out        create;
>    fuse_create_out_7_8    create_7_8;
>    /* The protocol places no limits on the size of bytes */
> -    uint8_t            bytes[0x20000];
> +    uint8_t        bytes[0x20000];

What does this hex value represent? Could it be made into a constant?

...

> -        cur_size = std::max(cur_size,
> +        cur_size = std::max((uint64_t)cur_size,
>            in->body.write.size + in->body.write.offset);
>    })));

Casting like this is legal, but this is the C way of doing casting. C++11 and newer recommends using static_cast for all compile time casting and dynamic_cast for all runtime casting.

Thanks :)!
-Enji