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

[Alan Somers]

On Sun, May 26, 2019 at 8:02 AM Enji Cooper <yaneurabeya at gmail.com> wrote:
>
>
> 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.

Done!  Nothing like using C++ smart pointers to make me miss Rust.

>
> }
>
> 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?

It's just "a big buffer" used for different purposes by different
tests.  The size is arbitrary.  I've tried to make that more clear in
the comment.

>
> ...
>
> -        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.

Done!  Here, at least.  In some other locations I've stuck with
C-style casting because it involves significantly fewer characters, so
I find it more readable.

>
> Thanks :)!
> -Enji

Since you're interested in C++ issues, would you mind taking a look at
a painful compiler error I'm getting?  This only happens with GCC-8,
not Clang, so I'm only seeing it in the riscv.riscv64 buildworld.

In file included from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/algorithm:645,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/__string:57,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/string_view:176,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/string:505,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/__locale:15,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/ios:216,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/ostream:138,
                 from
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/private/gtest/gtest.h:56,
                 from
/home/asomers/freebsd/base/projects/fuse2/tests/sys/fs/fusefs/mockfs.cc:53:
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/functional:
In instantiation of 'std::__1::__function::__value_func<_Rp(_ArgTypes
...)>::__value_func(_Fp&&, _Alloc) [with _Fp =
ReturnImmediate(std::__1::function<void(const mockfs_buf_in&,
mockfs_buf_out&)>)::<lambda(auto:5&, auto:6&)>; _Alloc =
std::__1::allocator<ReturnImmediate(std::__1::function<void(const
mockfs_buf_in&, mockfs_buf_out&)>)::<lambda(auto:5&, auto:6&)> >; _Rp
= void; _ArgTypes = {const mockfs_buf_in&,
std::__1::vector<std::__1::unique_ptr<mockfs_buf_out,
std::__1::default_delete<mockfs_buf_out> >,
std::__1::allocator<std::__1::unique_ptr<mockfs_buf_out,
std::__1::default_delete<mockfs_buf_out> > > >&}]':
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/functional:2290:46:
  required from 'std::__1::function<_Rp(_ArgTypes ...)>::function(_Fp)
[with _Fp = ReturnImmediate(std::__1::function<void(const
mockfs_buf_in&, mockfs_buf_out&)>)::<lambda(auto:5&, auto:6&)>;
<template-parameter-2-2> = void; _Rp = void; _ArgTypes = {const
mockfs_buf_in&, std::__1::vector<std::__1::unique_ptr<mockfs_buf_out,
std::__1::default_delete<mockfs_buf_out> >,
std::__1::allocator<std::__1::unique_ptr<mockfs_buf_out,
std::__1::default_delete<mockfs_buf_out> > > >&}]'
/home/asomers/freebsd/base/projects/fuse2/tests/sys/fs/fusefs/mockfs.cc:149:3:
  required from here
/scratch/tmp/asomers/obj/home/asomers/freebsd/base/projects/fuse2/riscv.riscv64/tmp/usr/include/c++/v1/functional:1711:36:
error: placement new constructing an object of type '_Fun' {aka
'std::__1::__function::__func<ReturnImmediate(std::__1::function<void(const
mockfs_buf_in&, mockfs_buf_out&)>)::<lambda(auto:5&, auto:6&)>,
std::__1::allocator<ReturnImmediate(std::__1::function<void(const
mockfs_buf_in&, mockfs_buf_out&)>)::<lambda(auto:5&, auto:6&)> >,
void(const mockfs_buf_in&,
std::__1::vector<std::__1::unique_ptr<mockfs_buf_out> >&)>'} and size
'64' in a region of type 'std::__1::aligned_storage<24>::type' and
size '32' [-Werror=placement-new=]
                     ::new ((void*)&__buf_) _Fun(_VSTD::move(__f),
_Alloc(__af));
                                    ^~~~~~

Do you have any idea how to fix that?
-Alan