Skip to content

Fix/logging snprintf negative time 4556 #4593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tarek-y-ismail merged 10 commits into from
Jan 9, 2026
Merged

Fix/logging snprintf negative time 4556 #4593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
549b3c4
fix : fixes incorrect logging of future timestamps
DigraJatin Jan 2, 2026
d972dc0
added unit tests for negative timestamp logging
DigraJatin Jan 2, 2026
598d652
addressed code review comments
DigraJatin Jan 5, 2026
c9a57c1
pre-commit: apply automatic fixes
pre-commit-ci-lite[bot] Jan 6, 2026
8fedcd4
fixed failing unit tests on CI
DigraJatin Jan 6, 2026
90f7278
Merge branch 'fix/logging-snprintf-negative-time-4556' of https://git…
DigraJatin Jan 6, 2026
0e35fab
pre-commit: apply automatic fixes
pre-commit-ci-lite[bot] Jan 8, 2026
343fa3d
fix unit test for CI
DigraJatin Jan 8, 2026
51386ca
Merge branch 'fix/logging-snprintf-negative-time-4556' of https://git…
DigraJatin Jan 8, 2026
7084aee
Remove redundant `chrono_literals` use statement
tarek-y-ismail Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/common/logging/input_timestamp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <mir/logging/input_timestamp.h>
#include <cstdio>
#include <cstdlib>

using namespace std::chrono;
std::string mir::logging::input_timestamp(nanoseconds when)
Expand All @@ -26,9 +27,15 @@ std::string mir::logging::input_timestamp(nanoseconds when)
long long const now_ns = duration_cast<nanoseconds>(now).count();
long long const age_ns = now_ns - when_ns;

long long const abs_age_ns = std::llabs(age_ns);
long long const milliseconds = abs_age_ns / 1000000LL;
long long const sub_milliseconds = abs_age_ns % 1000000LL;

char const* sign_suffix = (age_ns < 0) ? "in the future" : "ago";

char str[64];
snprintf(str, sizeof str, "%lld (%lld.%06lldms ago)",
when_ns, age_ns / 1000000LL, age_ns % 1000000LL);
snprintf(str, sizeof str, "%lld (%lld.%06lldms %s)",
when_ns, milliseconds, sub_milliseconds, sign_suffix);

return std::string(str);
}
1 change: 1 addition & 0 deletions tests/unit-tests/logging/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
list(APPEND UNIT_TEST_SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/test_display_report.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test_compositor_report.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test_input_timestamp.cpp
)

set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)
51 changes: 51 additions & 0 deletions tests/unit-tests/logging/test_input_timestamp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright © Canonical Ltd.
*
* This program is free software: you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 or 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include <mir/logging/input_timestamp.h>

#include <gtest/gtest.h>
#include <gmock/gmock.h>
#include <chrono>
#include <string>

using namespace std::chrono;
using namespace std::chrono_literals;
using namespace testing;
namespace ml = mir::logging;

TEST(TimestampTest, past_time_is_correctly_formatted)
{
auto const past_event = steady_clock::now().time_since_epoch() - 500ms;

std::string out = ml::input_timestamp(past_event);

EXPECT_THAT(out, Not(HasSubstr(".-")));
EXPECT_THAT(out, HasSubstr("500.0"));
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test checks for an exact substring '500.0' which may be fragile if the implementation changes decimal precision or formatting. Consider using a more flexible pattern or checking just for '500' and 'ms ago' separately to avoid false negatives.

Copilot uses AI. Check for mistakes.
EXPECT_THAT(out, HasSubstr("ms ago"));
}

TEST(TimestampTest, future_time_is_correctly_formatted)
{
auto const future_event = steady_clock::now().time_since_epoch() + 1000ms + 10us;

std::string out = ml::input_timestamp(future_event);

EXPECT_THAT(out, Not(HasSubstr(".-")));
EXPECT_THAT(out, HasSubstr("in the future"));
EXPECT_THAT(out, HasSubstr("1000.0"));
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test checks for an exact substring '1000.0' which may be fragile if the implementation changes decimal precision or formatting. Consider using a more flexible pattern or checking just for '1000' and 'ms' separately to avoid false negatives.

Copilot uses AI. Check for mistakes.
EXPECT_THAT(out, HasSubstr("ms"));

}
Loading