Repository navigation
linux: render N/A instead of 0 for unreadable smaps metrics - #2138
ravi-arnan wants to merge 3 commits into
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Mark metrics unavailable when fgets() fails. · LinuxProcessTable.c:895
linux/LinuxProcessTable.c:895
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMark metrics unavailable when
fgets()fails.If a process exits after
fopenat()succeeds, the Linuxsmaps_rollupreader can returnESRCHduring the read. The loop then ends without checkingferror(fp). The fields retain zero or partial values instead ofULLONG_MAX. Checkferror(fp)after the loop and mark all four fields unavailable before closing the file. (raw.githubusercontent.com)
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
a8c6be93-9d2d-4344-a403-ae349bf3d0d2
📒 Files selected for processing: 7
Macros.hlinux/LinuxProcess.clinux/LinuxProcess.hlinux/LinuxProcessTable.cpcp/PCPProcess.cpcp/PCPProcess.hpcp/PCPProcessTable.c
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
|
Please split the commit into the general changes, the ones affecting Linux and those related to PCP. |
344a131 to
fcfa7c5
Compare
…west When a metric is unavailable (represented by a sentinel value like ULLONG_MAX), sorting descending should keep it at the bottom rather than ranking it as the largest value. Add SPACESHIP_NUMBER_NA() to handle this ordering. Signed-off-by: Ravi Arnan <[email protected]>
Reading /proc/<pid>/smaps_rollup requires PTRACE_MODE_READ. For processes belonging to another user, an unprivileged caller gets EACCES and fopenat() returns NULL. Because LinuxProcess is allocated with xCalloc(), the fields (m_pss, m_swap, m_psswp, m_epss) retained their 0 initialization, rendering 0 as if it were a measured value. Widen the smaps fields to unsigned long long to match Row_printKBytes(), initialize them to ULLONG_MAX, and set them to ULLONG_MAX on read failure (including partial reads where ferror() is set) so they render as N/A. Use SPACESHIP_NUMBER_NA() so that N/A ranks lowest in sort order. Closes htop-dev#1704 Signed-off-by: Ravi Arnan <[email protected]>
Widen the smaps fields to unsigned long long to match Row_printKBytes(), initialize them to ULLONG_MAX, and pass ULLONG_MAX as the fallback value to Metric_instance_u64() so unavailable smaps metrics render as N/A instead of 0. Use SPACESHIP_NUMBER_NA() so that N/A ranks lowest in sort order. Signed-off-by: Ravi Arnan <[email protected]>
fcfa7c5 to
cfe7fe6
Compare
|
Split into three commits per your request:
Each commit builds independently, and the final tree hash is unchanged. |
Closes #1704
Reading
/proc/<pid>/smaps_rollup(or/proc/<pid>/smaps) requiresPTRACE_MODE_READ. For processes owned by another user, an unprivileged caller getsEACCES.In
LinuxProcessTable_readSmapsFile(), whenfopenat()returnedNULL, the function returnedfalsewithout modifyingprocess->m_pssand friends. BecauseLinuxProcessis allocated withxCalloc(), those four fields (m_pss,m_swap,m_psswp,m_epss) retained their zero initialization. htop then rendered that0as if it were a valid measured value, whileRES(from world-readablestatm) was read normally.0 is a measurement; "cannot read this" is not.
The change
m_pss,m_swap,m_psswp,m_epssfromlongtounsigned long longinLinuxProcess.h(and matchingPCPProcess.h), consistent withRow_printKBytes().ULLONG_MAXinLinuxProcess_new()(andPCPProcess_new()). Whenfopenat()orskipEndOfLine()fails inLinuxProcessTable_readSmapsFile(), the fields are explicitly set toULLONG_MAX.Row_printKBytes()already rendersULLONG_MAXas shadowedN/A.SPACESHIP_NUMBER_NA(a, b, na)inMacros.hso thatN/A(ULLONG_MAX) is ranked lowest. BecauseM_PSSand friends have.defaultSortDesc = true, a plain maximum sentinel would float unreadable processes to the top when sorting descending. WithSPACESHIP_NUMBER_NA, unreadable processes sink to the bottom on descending sort and float to the top only on ascending sort, matching howNaN/N/Abehaves for percentage columns.strtoltostrtoullinLinuxProcessTable_readSmapsFile()to match the widened unsigned type.Verification
Tested on NixOS (Linux 6.17.0-35, glibc, unprivileged user
ravi):Before:
After (descending sort
PSS▽):After (ascending sort
PSS△):Across all processes on descending sort, active memory consumers sort from largest to smallest, while all unreadable processes (
N/A) sink to the bottom.