Skip to content

Commit

Permalink
Remove add_object_vtable expect from fanpwm UT
Browse files Browse the repository at this point in the history
For some reason, having an expect on the sdbusplus mock
sd_bus_add_object_vtable causes valgrind to fail in CI with errors such
as:

```
==729843== Conditional jump or move depends on uninitialized value(s)
==729843==    at 0x5CB89AB: std::unique_ptr<sd_bus_slot, sdbusplus::slot::details::SlotDeleter>::~unique_ptr() (unique_ptr.h:403)
==729843==    by 0x5CB8665: sdbusplus::slot::slot::~slot() (slot.hpp:43)
==729843==    by 0x5CC3CC9: sdbusplus::server::interface::interface::~interface() (interface.cpp:38)
==729843==    by 0x13189B: sdbusplus::server::xyz::openbmc_project::control::FanPwm::~FanPwm() (server.hpp:32)
==729843==    by 0x13E267: sdbusplus::server::object::details::compose<sdbusplus::server::xyz::openbmc_project::control::FanPwm>::~compose() (object.hpp:181)
==729843==    by 0x13E4BC: sdbusplus::server::object::object<sdbusplus::server::xyz::openbmc_project::control::FanPwm>::~object() (object.hpp:95)
==729843==    by 0x13C5C9: hwmon::FanPwm::~FanPwm() (fan_pwm.hpp:18)
==729843==    by 0x12C0D1: FanPwmTest_BasicConstructorDeferredTest_Test::TestBody() (fanpwm_unittest.cpp:76)
==729843==    by 0x5E089FC: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2638)
==729843==    by 0x5DFE67A: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest.cc:2674)
==729843==    by 0x5DD3BA3: testing::Test::Run() (gtest.cc:2713)
==729843==    by 0x5DD4661: testing::TestInfo::Run() (gtest.cc:2859)
```

There are 4 more stanzas of similar 'Conditional jump or move...'
errors.  None of these show up without the vtable expect.

Other repositories that use this same expect also fail.

I opened sdbusplus issue openbmc/sdbusplus#97 for the problem.  I spent
time looking into it, and did not figure anything out.

This commit just removes the call to it because in these testcases the
FanPwm object, which is just derived from the sdbuplus interface object,
is simply created on the stack:

```
hwmon::FanPwm f(...);
```

The unit tests aren't actually testing any code that conditionally
creates a FanPwm interface on D-Bus, so checking that it happens isn't
that useful and seems to more just be testing sdbusplus.

Change-Id: Idf2df085bf909af1ad94ba06863c4605831a3824
Signed-off-by: Matt Spinler <[email protected]>
  • Loading branch information
spinler committed Jul 23, 2024
1 parent 8df95be commit 82f2db6
Showing 1 changed file with 0 additions and 5 deletions.
5 changes: 0 additions & 5 deletions test/fanpwm_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ static auto FanPwmProp = "Target";
void SetupDbusObject(sdbusplus::SdBusMock* sdbus_mock, const std::string& path,
const std::string& intf, const std::string property = "")
{
EXPECT_CALL(*sdbus_mock,
sd_bus_add_object_vtable(IsNull(), NotNull(), StrEq(path),
StrEq(intf), NotNull(), NotNull()))
.WillOnce(Return(0));

if (property.empty())
{
EXPECT_CALL(*sdbus_mock,
Expand Down

0 comments on commit 82f2db6

Please sign in to comment.