Generalised the library API tests and created a common
test-runner. Instead of copy and pasting the same code in
each time.
First cut of pids API test, for _new so far.
Remove a remnant of this new API's evolution. Here we
no longer fill stacks, rather we reap and select them.
Signed-off-by: Jim Warner <james.warner@comcast.net>
By co-mingling both external/internal identifiers with
actual implementation code, potential future additions
to our API would have been considerably more difficult.
So, this patch will now rely solely on internal/hidden
identifiers serving as fenceposts in validation logic.
And if the following convention is used for new fields
we will maintain that enumerator alphabetic ordering a
a little longer (even though 2 user fields now don't):
. PROCPS_PIDS_XTRA_FOO_A, PROCPS_PIDS_XTRA_FOO_B, etc.
Reference(s):
http://www.freelists.org/post/procps/me-too-newlib,7
Signed-off-by: Jim Warner <james.warner@comcast.net>
Unfortunately, reading that '/proc/sys/kernel/pid_max'
file returns a newline, which we will now account for.
[ also, we should use the existing dedicated buffer ]
Reference(s):
. original 'procps_pid_length' introduction
commit ccb6ae8de1
Signed-off-by: Jim Warner <james.warner@comcast.net>
escaped_copy(): only appears in ps, moved to ps/output.c
escape_strlist() only used in escape.c made static
escape_command() used in library, made internal
procps.h no longer includes escape.h
escape_str() used by library and ps so needs to be exported
definition put into procps.h including the odd define required.
Far from ideal to have it this way, will look at it another time
to have it all in, all out or split nicer so its not in the API;
perhaps a lib/ file?
readproc still had some of the old API hanging around that
was good while we were trying it out, but its time to say
goodbye:
readproc.h removed from public procps.h header file
enum ns_type - not used - removed
get_ns_name() - not defined - removed
get_ns_id() - not defined - removed
PROCTAB proc_t only used internal to library
readproctab() and 2,3 friends not used - removed
proc_data_t was used by readproctab23() - removed
readtask() - not used - removed
read_cmdline() - copy,renamed and made generic - remove original
freeproc() - not used - removed
get_proc_status - not used - removed, however there should be
a new function created that does this. Given a PID return data
about it instead of scanning the entire procfs. Maybe it already does.
Left as internal-to-library only functions:
readproc(), readeither(), look_up_our_self(), openproc(), closeproc()
Updated libprocps.sym to export only what we use.
Now that the conditional OOMEM_ENABLE has been removed
and, after reviewing current library support, it turns
out we've been using a deprecated /proc/<pid>/oom_adj.
What we should use instead is that more recent tunable
/proc/<pid>/oom_score_adj. This new field will provide
a range of -1000 thru +1000 (former was -17 thru +15).
Reference(s):
. removal of misguided OOMEM_ENABLE
commit 64238730fa
. linux, partial oom_adj revert (Nov, 2012)
commit fa0cbbf145aabbf29c6f28f8a11935c0b0fd86fc
. linux, removal oom_adj (Oct, 2012)
commit 01dc52ebdf472f77cca623ca693ca24cfc0f1bbe
Signed-off-by: Jim Warner <james.warner@comcast.net>
The includes used to define a lot of things a library include
should not. It was also a bit messy what was exposed in the library
and what was not.
get_pid_digits -> procps_pid_length and exported correctly
MALLOC attribute move into relevant .c files
NORETURN attribute moved to relevant .c, not used in library
PURE attribute removed, it wasn't used
KLONG/KLF/STRTOUKL were fixed for long, so now just use long
HIDDEN attribute removed. It was for 3 functions. The PROCPS_EXPORT
seems to do the same (opposite) thing.
likely/unlikely removed from most places, its highly debateable
this does anything useful as CPUs have gotten smarter about branches.
Re-arranged the includes, ALL external programs should just #include
<proc/procps.h> then proc/procps.h includes headers for files that
have exported functions. procps.h and the headers it includes should
not use items that are not exportable (e.g. hidden functions or
macros) they go in procps-private.h
Multiple scanf()s use the GNU-permitted %Lu. This is not supported in
other libraries and isn't to the POSIX specification. The L modifier
is only used for floats in POSIX.
Replacing %Lu with %llu is the same for GNU libc (scanf(3) says as much)
but means other libraries will work fine.
From master commit da715e3
References:
http://pubs.opengroup.org/onlinepubs/009695399/functions/fscanf.html
Beginning with linux-4.5, the following new fields are
being added under that /proc/<pid>/status pseudo file:
. RssAnon - size of resident anonymous memory
. RssFile - size of resident file mappings
. RssShmem - size of resident shared memory
This patch just represents the initial library and top
support, sharing a commit message with 2 more patches.
p.s. locked resident memory support was also added but
isn't directly related to the kernel 4.5 enhancements.
Reference(s):
commit 1f8e41d019
Signed-off-by: Jim Warner <james.warner@comcast.net>
Summarized below, miscellany addressed in this commit:
. deleted extraneous newline(s) for consistent spacing
Signed-off-by: Jim Warner <james.warner@comcast.net>
When reference counts were added to some string fields
the 3 true string vector fields were not duplicated as
were those other fields. Instead they were supposed to
disallow a duplicate stack reference beyond the first.
However, the actual implementation gave NULL for every
true vector field whenever such items were duplicated.
More importantly, such true string vector fields never
considered references to the shared proc_t source root
which would have forced the conversion of such vectors
into a single string form via the '_CVT' library flag.
So this commit restores the intended outcome with true
string vectors. There's only 1 valid reference allowed
and duplicates and converted fields will yield a NULL.
Signed-off-by: Jim Warner <james.warner@comcast.net>
While not changing generated code this commit corrects
one free reference from 'str' to a more proper 'strv'.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Ok, here is that rather major internal redesign hinted
at in the three previous commits. Its need was quickly
revealed after adapting top then attempting to display
newly added 'CGNAME' fields and an existing 'CGROUPS'.
That very quickly generated a SEGV. And the reason was
just as quickly recognized. Both fields relied on that
proc_t.cgroup member yet whichever result structure is
first in a stack is the one which assumes ownership of
of the vectored sting by resetting its cgroup to NULL.
So this commit introduces reference counting for a few
of the fields in the proc_t. Specifically there are 17
entries in the Item_table dealing with strings/vectors
where ownership is transferred to newlib. Now whenever
such fields are represented more than once in a stack,
the strings will be duplicated instead of transferred.
In this way we can generally remain optimized avoiding
string copies, yet still accommodate them when needed.
There's an exception to this scheme: those true string
vectors (CGROUP_V, CMDLINE_V and ENVIRON_V). When such
fields are duplicated in a stack the result structures
beyond the first will be set to NULL, which the caller
will (should) already be equipped to deal with anyway.
Signed-off-by: Jim Warner <james.warner@comcast.net>
The ps program was modified to print the control group
names, based on the library provided list of all those
control groups to which a process belongs. But this is
probably something the newlib should be doing for all.
So this commit borrows the ps approach to cg names and
thus will make that available to all future consumers.
[ but stay tuned! there is a commit coming soon that ]
[ represents a rather major internal redesign, which ]
[ was prompted by the ps and top adaptation testing. ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
The above function was the sole public function in the
<pids> interface to use the word 'stacks' in its name.
All of the others dealt exclusively with their duties,
So this commit normalizes that outlier by renaming it.
Signed-off-by: Jim Warner <james.warner@comcast.net>
The above function had been disabled via '#if 0' so as
to prevent a compiler warning. But it really should be
called by that 'procps_pids_read_shut' function rather
than it duplicating/reinventing the same logic itself.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Gosh, just because nobody uses some newlib provision I
guess, since it is being offered, it ought to actually
be tested at some point. Well, that point just arrived
and guess what? A surprise: some bugs were discovered.
The procps_pids_select function established a for loop
wherein readproc is called until the passed 'maxthese'
limit. Unfortunately this was incorrect for 2 reasons:
1. For PROCPS_FILL_PID results are limited by what the
oldlib finds, having established the pid list at open.
Total found already cannot exceed a passed 'maxthese';
2. With PROCPS_FILL_UID, returned results could exceed
a 'maxthese' thus making the for loop incorrect again.
[ plus yours truly neglected to forward the required ]
[ UIDs total to our old library, another oops biggie ]
In summary: the loop should have been forever, exiting
only when all those identified procs had been located.
So, while addressing those bugs, I've consolidated all
the retrieval code (initialize, iterate, summarize) in
a single helper function which will now serve both the
procps_pids_reap and select functions. And as a result
those guys were reduced to quite trivial housekeeping.
This patch, hopefully, completes the normalization for
reap/select (fill), which began with references shown.
Reference(s):
commit 0c953eccc5
commit 747dfc5987
Signed-off-by: Jim Warner <james.warner@comcast.net>
This patch contains the following minor modifications:
. When assigning to boot_seconds from a double, a cast
to 'unsigned long' was employed that in reality should
have been 'unsigned long long'. But, on second thought
it's probably best if the compiler makes the decision.
. Results for ID_TGID do not require an 'f_status' flg
since both tid and tgid will be available by virtue of
the 'simple_nextpid' function normal operations. Thus,
that flag has been removed from the <pids> Item_table.
. When the pids_stacks structure was eliminated with a
change to the alloc/dealloc functions in the reference
below, several casts became redundant and are removed.
Reference(s):
commit 747dfc5987.
Signed-off-by: Jim Warner <james.warner@comcast.net>
After simplifying that select/fill interface, there is
no longer a need for public 'alloc' & 'dealloc' stacks
functions. There is now only one instance of stacks as
an input parameter found in procps_pids_stacks_sort().
But sorting 'empty' stacks serves no possible purpose.
So this commit retains both functions, since they will
still be needed, but designates them private (static).
Additionally, with their demise we will eliminate that
pids_stacks structure from the header file, internally
using what always was the true 'stacks_extent' struct.
Signed-off-by: Jim Warner <james.warner@comcast.net>
After wrestling with the conversion of both top and ps
the differences between reap (all) & fill (select) has
become increasingly inconvenient. So this patch simply
normalizes that API making returned results identical.
The former procps_pids_stacks_fill identifier will now
be known as procps_pids_select which serves as logical
counterpart to the existing procps_pids_reap function.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Shoot, here's yet another bow to ps needs. But it's ok
because it makes a lot of sense. Rather than force all
users into their own calculations do but it once here.
As an aside this need arose during ps testing when the
sorts were using TIME_START or TICS_ALL. That was just
fine for almost every need except 'etime' plus 'time'.
That 'etime' was sorting the opposite of what's wanted
when using TIME_START (of course) while 'time' yielded
some weird ordering because TICS_ALL was too granular.
Signed-off-by: Jim Warner <james.warner@comcast.net>
While not documented in the man page, ps allows 'tty4'
as a valid output specifier complimenting 'tty8' & its
derivatives. So, in order to eliminate a dev_to_name()
call in the ps program the library will now offer this
abbreviated tty version (consisting of a number only).
Signed-off-by: Jim Warner <james.warner@comcast.net>
The 'PROCPS_PIDS_extra' enumerator already enjoys some
special place wherein it's zeroed with each iteration.
This patch simply extends the special handling to also
include support for sorting. It will be treated as the
'ull_int' data type, since that encompasses the entire
scope of that union within all pids_result structures.
[ plus, we've also corrected the actual 'extra' name ]
This change was prompted by the conversion of ps which
needs that enumerator to store the former 'pcpu' data,
so it will be available for sorting (not for display).
Signed-off-by: Jim Warner <james.warner@comcast.net>
The way that the passed sort order was validated would
allow the invalid 0 to fall between the sofa cushions.
So this patch will simply close that former oversight.
Signed-off-by: Jim Warner <james.warner@comcast.net>
As part of the push to remove the old API, 3 more old calls were
deleted from the library. Now all someone needs to do is fix ps
and we're largely done.
The values of PROCPS_SORT_ASCEND & PROCPS_SORT_DESCEND
were a tad unintuitive. This patch will just make them
a more natural +1 for ascending and -1 for descending.
[ plus it still allows that fast path multiplication ]
[ instead of a comparison for signed numbers/strings ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
After peeking at possible conversion of the ps program
it appeared that we may ultimately need the concept of
a 'static' pids_stack in suport of look_up_our_self().
In other words, one not impacted by procps_pids_reset.
This patch simply sets the stage for that possibility.
Signed-off-by: Jim Warner <james.warner@comcast.net>
With this patch the distinction between a 'long' KLONG
and a 'long long' KLONG is being abandoned in favor of
a consistent declaration as 'long' only. Plus we would
have also defined it as 'unsigned' except there exists
much code already explicitly specifying the qualifier.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Work on converting ps has revealed the desirability of
trading a void pointer for that ul_int type. There was
much arithmetic employed against such values and casts
would otherwise have been required. Even pmap needed a
cast on occasions when comparing an internal variable.
Besides, there is much to be said for reducing demands
on (and the complexity of) the result structure union.
[ we choose ul_int over ull_int since that former is ]
[ the exact same size and capacity as a void pointer ]
[ regardless of whether compiled as 32-bit or 64-bit ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
Not sure how this one has gone unnoticed until now but
with valgrind's help it's going bye-bye lickety-split.
Reference(s):
==26533== Conditional jump or move depends on uninitialised value(s)
==26533== at 0x4E4082B: procps_meminfo_stack_fill (meminfo.c:408)
Signed-off-by: Jim Warner <james.warner@comcast.net>
[ but most definitely not all of them by a long shot ]
Reference(s):
proc/diskstat.c:186:17: warning: unused variable 'is_disk' [-Wunused-variable]
int retval, is_disk;
^
proc/namespace.c:110:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^
proc/readproc.c:1131:50: warning: address of array 'ent->d_name' will always evaluate to 'true' [-Wpointer-bo
if(unlikely(unlikely(!ent) || unlikely(!ent->d_name))) return 0;
~~~~~~^~~~~~
proc/readproc.c:1158:50: warning: address of array 'ent->d_name' will always evaluate to 'true' [-Wpointer-bo
if(unlikely(unlikely(!ent) || unlikely(!ent->d_name))) return 0;
~~~~~~^~~~~~
proc/sysinfo.c:45:12: warning: unused variable 'stat_fd' [-Wunused-variable]
static int stat_fd = -1;
^
proc/sysinfo.c:49:12: warning: unused variable 'meminfo_fd' [-Wunused-variable]
static int meminfo_fd = -1;
^
proc/sysinfo.c:51:12: warning: unused variable 'vminfo_fd' [-Wunused-variable]
static int vminfo_fd = -1;
^
proc/sysinfo.c:53:12: warning: unused variable 'vm_min_free_fd' [-Wunused-variable]
static int vm_min_free_fd = -1;
^
proc/uptime.c:157:12: warning: unused variable 'realseconds' [-Wunused-variable]
time_t realseconds;
^
proc/uptime.c:158:16: warning: unused variable 'realtime' [-Wunused-variable]
struct tm *realtime;
^
vmstat.c:574:20: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]
DSTAT(PROCPS_DISKSTAT_READ_TIME),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vmstat.c:578:20: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]
DSTAT(PROCPS_DISKSTAT_WRITE_TIME),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
watch.c:230:7: warning: variable 'endptr' is uninitialized when used here [-Wuninitialized]
if (*endptr == '\0') set_ansi_attribute(0); /* [m treated as [0m */
^~~~~~
Signed-off-by: Jim Warner <james.warner@comcast.net>
OK, ok, this was kind of a huge omission. So please do
not select the TTY field for display in top quite yet,
at least until a next patch has been pushed to GitLab.
And to produce a correct sort order for this new field
the GNU 'strverscmp' routine was a necessary addition.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Uptime output for both w and uptime command were showing no
comma or space after days.
$ ./uptime
18:32:21 up 22 days7 min, 6 users, load average: 0.23, 0.46, 0.64
Minor tweak to fix this.
This commit is an outgrowth of the research into a bug
that recently surfaced with the 'w' program. And while
that program was just a victim several inconsistencies
were found in the handling of library flags during the
research. This patch just address such irregularities.
Reference(s):
http://www.freelists.org/post/procps/newlib-at-the-precipice,4
Signed-off-by: Jim Warner <james.warner@comcast.net>
I can't remember during what sleepy hour that enum was
added to pids.h, but it damn sure fell short of a full
implementation by approximately 1,000,000 miles or so.
Strangely it surfaced on Craig's system seemingly ggdb
related whereas on mine, a segmentation fault was only
seen when that -h (no header) argument was being used.
Oh well, the road to its resolution also offered us an
opportunity to cleanup some other items along the way.
Reference(s):
http://www.freelists.org/post/procps/newlib-at-the-precipice,4
Signed-off-by: Jim Warner <james.warner@comcast.net>
The former whattime logic used to suppress that 'days'
output when the system had been up less than 24 hours.
But since the refactor for newlib, '0 days' was always
output under those conditions. So this commit restores
the former expected behavior of suppressing that item.
[ plus an erroneous calculation of uphours was fixed ]
[ and the clang warnings shown below were also fixed ]
Reference(s):
proc/uptime.c:74:10: warning: unused variable 'buf' [-Wunused-variable]
char buf[256];
^
proc/uptime.c:131:58: warning: data argument not used by format string [-Wformat-extra-args]
pos += sprintf(upbuf + pos, "%d min, ", uphours, upminutes);
~~~~~~~~~~ ^
proc/uptime.c:175:15: warning: expression result unused [-Wunused-value]
comma +1;
~~~~~ ^~
Signed-off-by: Jim Warner <james.warner@comcast.net>
The presence of that PROCPS_PIDS_noop may yet see some
use in the future with its 'no alter' library promise.
However, when top used that item to reflect the forest
view nesting level, the unchanging nature of that item
became more of an inconvenience than benefit. For each
refresh top was forced to loop through all the stacks,
resetting that PROCPS_PIDS_noop result struct to zero.
So this commit will now offer users a choice between a
new re-initialized item (PROCPS_PIDS_extra) & the noop
invariant. Since the library already resets all those
result structures, top will now utilize it at no cost.
Signed-off-by: Jim Warner <james.warner@comcast.net>
A patch containing the following miscellaneous tweaks:
. make a supposedly robust parameter test truly robust
[ ensure the largest enum value used with validation ]
. remove duplicate item test in cleanup_stack function
[ is already subordinate to test of PROCPS_PIDS_noop ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
Functions related to namespaces were half-in half-out of the
procps library and didn't fit the standard naming scheme.
While struct { long ns[x]} is a bit clunky, its the only way
to "lock in" x. The alternative is to use ns_* variables.
This work was needed before pgrep could be converted.
I was surprised to find that ol' gcc silently converts
a single (different) enum into an address where one or
more enums were expected to be dereferenced. Of course
this was just yet another way to generate an old SEGV.
So this commit will strengthen those parameter checks.
[ we will *not* blame Craig for a failure to consult ]
[ the documentation, since it doesn't even exist yet ]
Reference(s):
http://www.freelists.org/post/procps/newlib-ps-fix,8
Signed-off-by: Jim Warner <james.warner@comcast.net>
A patch containing the following miscellaneous tweaks:
. avoided distortions unique to PROCPS_PIDS_TICS_DELTA
. addressed several smatch warnings and/or suggestions
. ensured oldproc_close invoked should tally_proc fail
. keeping that namespace clean, added a missing #undef
. added 2 comments acknowledging pids_item as unsigned
. added/clarified comments regarding proc flags & strv
From smatch analysis:
. these were indeed boo-boos
pids.c:580 make_hist() warn: the 'Hr' macro might need parens
pids.c:1058 procps_pids_reap() warn: variable dereferenced before check 'info' (see line 1056)
. these were not errors (and we did double check)
pids.c:1067 procps_pids_reap() warn: double check that we're allocating correct size: 8 vs 128
pids.c:1068 procps_pids_reap() warn: double check that we're allocating correct size: 8 vs 128
Signed-off-by: Jim Warner <james.warner@comcast.net>
As an experiment a helper macro used to extract values
from a result stack has been added to the header file.
Don't force callers to reinvent that particular wheel.
Signed-off-by: Jim Warner <james.warner@comcast.net>
