Skip to content

fix: increase buffer size for elapsed time on timeline#479

Open
aadamowski wants to merge 1 commit into
Syllo:masterfrom
aadamowski:master
Open

fix: increase buffer size for elapsed time on timeline#479
aadamowski wants to merge 1 commit into
Syllo:masterfrom
aadamowski:master

Conversation

@aadamowski

Copy link
Copy Markdown

Description

This pull request fixes a bug in the nvtop timeline where the elapsed time would erroneously display as err when the terminal width was large and the update delay was significant.

Problem

When running nvtop in a wide terminal with a large update delay, the timeline would sometimes show err instead of the actual elapsed time (e.g., 1234s).

This occurred because the buffer used to format the elapsed time string was too small. When the number of seconds exceeded the capacity of the buffer, snprintf would return a value indicating truncation, which the code interpreted as an error, triggering the fallback to the "err" string.

Root Cause

In src/interface.c, the buffer for the elapsed time string was defined with a hardcoded size of 5:

char elapsedSeconds[5];

As the application ran longer, the number of seconds would eventually require more than 4 characters (plus the null terminator). For example, "100s" is 4 characters, and "1000s" is 5 characters. When snprintf attempted to write a string longer than the buffer, it returned a value $\ge 5$, which matched the following error check:

if (retval > 4)
  toPrint = err;

Fix

The fix involves three key changes in src/interface.c:

  1. Increased Buffer Size: Expanded the elapsedSeconds buffer from 5 to 16 characters to safely accommodate much longer time durations.
  2. Robust Length Checking: Updated the snprintf return value check to use sizeof(elapsedSeconds) instead of the hardcoded 4. This ensures that the error handling logic scales correctly with the buffer size.
  3. Type Safety: Added an explicit cast to (int) for the sizeof comparison to avoid signed/unsigned comparison warnings.

Verification Results

The fix was verified using the following steps:

  1. Reproduction: A test was conducted by setting a large terminal width and a significant update delay, then checking the output for the error string using the following command:

    (export COLUMNS=320; printf '\033[21~' | nvtop --no-color) | strings -n 3 | grep -E 'err|[0-9]{2,}s$'

    The command successfully detected the "err" string in the output, reproducing the bug.

  2. Build: The nvtop binary was rebuilt from source.

  3. Validation: The same test command was re-run. The command no longer detected "err" and instead confirmed that the output contains valid elapsed time strings (e.g., 1570s). The test was repeated with increasingly large values of delay - including the max value of 99.9 seconds that is settable from the UI (UpdateInterval = 99900 in ~/.config/nvtop/interface.ini) and even larger values set directly in the config file (up to 86400 seconds, which corresponds to a full day long update interval).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant