top(8): fix process state sorting
ClosedPublic
Actions

Authored by kib on Jun 19 2023, 4:04 PM.

Details

Reviewers

Commits

rG6a5e6a50bd55: top(8): add missed SLOCK state to the sorted_state[]
rGd636fc5bd1e2: top(8): document the actual sort order for process sorting
rGbc2ac2585aa8: top(8): use designated initializers for sorted_state[]
rG5c6393451f58: top(8): designate the sorted_state[] array as constant

Summary

top(8): designate the sorted_state[] array as constant

top(8): use designated initializers for sorted_state[]

top(8): document the actual sort order for process sorting

    top(8): add missed SLOCK state to the sorted_state[]
    
    Put it near/after the sleep state.
    
    Noted by:       Mark Millard <marklmi@yahoo.com>
`

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Jun 19 2023, 4:04 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 19 2023, 4:04 PM

kib requested review of this revision.Jun 19 2023, 4:04 PM

kib edited the summary of this revision. (Show Details)

kevans accepted this revision.Jun 19 2023, 4:14 PM

kevans added inline comments.

usr.bin/top/machine.c
1265	With designated initializers we could just drop the unused 0 now, no? Holes get filled in with 0 anyways, and I'm not sure the documentation of it as unused is necessarily useful.
1292	I wonder if we should drop some assertions here to catch a `ki_stat >= nitems(sorted_state)` rather than relying on the out of bounds access to be caught in some other fashion, but I don't insist...

This revision is now accepted and ready to land.Jun 19 2023, 4:14 PM

Drop [0] initializer.

This revision now requires review to proceed.Jun 19 2023, 4:40 PM

kib added inline comments.Jun 19 2023, 4:43 PM

usr.bin/top/machine.c
1292	My HPoV there is that the asserts abruptly stopping the utility are worse then accessing random memory and sorting processes less precisely. I do not believe that compilers can turn this UB into rm -rf (yet). Having random user visible failure for something that is just a display is annoying.

kevans accepted this revision.Jun 19 2023, 4:53 PM

kevans added inline comments.

usr.bin/top/machine.c
1292	Reasonable

This revision is now accepted and ready to land.Jun 19 2023, 4:53 PM

markmi_dsl-only.net added a subscriber: markmi_dsl-only.net.Jun 19 2023, 5:27 PM

markmi_dsl-only.net added inline comments.

usr.bin/top/machine.c
1271	The comments only seem to make sense for: [SZOMB] <=> zombie [SLOCK] <=> blocked on lock. Those comments do not add as much now, given the new notation. But things like: [SRUN] <=> interrupt wait just seem misleading.

Update comments for the state descriptions.

This revision now requires review to proceed.Jun 20 2023, 3:54 AM

netchild added a subscriber: netchild.Jun 20 2023, 5:51 AM

netchild added inline comments.

usr.bin/top/machine.c
1292	Printing a one time warning instead of an assert? It's not uncommon that the output of top is garbled by whatever reason in real-world usage, and I use ctrl-l to fix the messed up display when it happens. As such I would not mind if it is a simple one-time printf, but can understand if someone would consider this too ugly.