With an eye to the future, rather than managing a long
switch/case construct in the procps_stat_get function,
we'll adopt the approach used in the <meminfo> module.
By making 'get' processing table driven too, just like
'select' is already, that may ease future maintenance.
Along the way a rather large boo-boo was fixed dealing
with that /proc/stat 'procs_running' field which crept
in with the commit referenced below. It wasn't spelled
correctly and thus was never captured as PROC_RUNNING.
Reference(s):
commit abc71a46ad
Signed-off-by: Jim Warner <james.warner@comcast.net>
When this interface was normalized/standardized, under
the commit referenced below, the parameters were never
validated in the 'get' function. Let's plug that hole.
Reference(s):
commit 407f1b71de
Signed-off-by: Jim Warner <james.warner@comcast.net>
Update the pids tests for tweaked API. Unsure why the toomany
test doesn't work, may point to a problem in the change as it used
to trigger, or the library has flexibility.
In that reference below a specific systemd problem was
fixed in the commit shown. However lurking deep within
the <pids> interface was yet one final case where NULL
could be returned, involving 'strv' and the following:
. a user requested both a single string vector (always
returned as a normal string) & the vectorized version,
as with PROCPS_PIDS_CMDLINE and PROCPS_PIDS_CMDLINE_V.
. a user simply duplicated some vectorized enum items.
The root of that NULL problem is the fact those single
string vectors shared the same proc_t field with their
true vectorized version. So while multiple occurrences
for most strings could be satisfied with strdup versus
the normal ownership usurpation, those true vectorized
fields could not be quite so easily copied/duplicated.
Thus newlib chose to return a NULL result.strv pointer
under either of the above scenarios (which perhaps was
just a user boo-boo in the first place). In any event,
the NULL was a potential for true string vectors only.
Now, since newlib is the sole caller into the readproc
module, separate fields have been created for what are
just normal strings (never vectorized) and those which
remain the true vectorized versions. And, former flags
which only worked if combined, now act as stand alone.
Thus, both PROCPS_PIDS_CMDLINE & PROCPS_PIDS_CMDLINE_V
can be used simultaneously (as they should have been).
Also with this patch, items which a user duplicates in
the stack (beyond the first such item) will return the
the string "[ duplicate ENUM_ID ]". This practice will
apply to both single strings and true vectorized ones.
In addition to informing users of their error, it will
also mean potential NULLs need now never be a concern.
Reference(s);
http://www.freelists.org/post/procps/systemd-binary-vs-library
commit 0580a7b4c6
Signed-off-by: Jim Warner <james.warner@comcast.net>
Calls to free() have now been reintroduce in the new()
function to quiet coverity warnings. Those free's were
removed originally as that library 'new' was returning
with a fatal error and a caller should end abnormally.
Plus, it is virtually impossible to fail a malloc call
under linux. And lastly, they required braces with the
if statement making the code considerably less pretty.
[ commit also includes 2 unrelated whitespace tweaks ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
A potential conflict arises should should some program
attempt to access systemd data using our library built
without that support (--disable-systemd). And while we
could argue that all callers should check against NULL
string pointers, it's not expected of libprocps users.
So we'll guarantee 'em valid string consisting of "?".
Reference(s):
http://www.freelists.org/post/procps/systemd-binary-vs-library
Signed-off-by: Jim Warner <james.warner@comcast.net>
Rather than adopt an approach offered below, involving
changing all enumerator names to convey types, perhaps
this much more modest change is a worthwhile 1st step.
By just changing the comments to 'right justification'
it dramatically improves readability, at least it does
to my eye. Perhaps such an approach can postpone that
massive alternative effort for the foreseeable future.
Reference(s):
http://www.freelists.org/post/procps/newlib-drip-drip,4
Signed-off-by: Jim Warner <james.warner@comcast.net>
This is just beginning an effort to minimize/normalize
the sheer variety of results types in use for our API.
In taking these first baby steps, a few anomalies were
found. There'll no doubt be many more yet to discover.
. the _FLT_ (fault) fields were already signed long in
the proc_t (even though their sscanf format used %lu).
. although strtoul will alway return an unsigned long,
all of the _VM_ fields were made signed long just like
other memory fields (& signed sorts more efficiently).
Reference(s):
http://www.freelists.org/post/procps/newlib-drip-drip,4
Signed-off-by: Jim Warner <james.warner@comcast.net>
This represents a rather major interface redesign. The
following highlights most of the changes/enhancements.
. The 'read' interface (employed by pgrep & pidof) saw
the biggest change. The 'open', 'next' and 'shut' guys
all went bye-bye, replaced by a single 'get' function.
. The items specified at 'new' time no longer serve as
the maximum. In fact, items & numitems are now treated
as optional, should callers prefer to wait until later
when the 'reset' function would then become mandatory.
. Even at 'reset' time, the stacks are not tied to any
sort of maximum. They will grow dynamically as needed.
. The order of some parameters was changed to parallel
that found in our other APIs. Specifically, when items
& numitems are needed they're specified in that order.
. A user will no longer be prevented from concurrently
employing any accessor functions. In other words, that
'get' (old 'read') won't preclude 'reap' and 'select'.
. A duplicate enumerator was found dealing with locked
resident pages. So, the name VM_LOCK was eliminated in
favor of VM_RSS_LOCKED, which is way more descriptive.
. The struct address returned to callers following any
reap() or select() is now more sharable as pids_fetch.
. Some input parameter names were changed to make them
more descriptive of the intended purpose/requirements.
------------------------------------------------------
Internally, there were numerous implementation changes
made that did not directly impact any potential users.
. That #define FPRINT_STACKS was eliminated along with
the associated supporting function and its invocation.
. Addresses returned following 'reap' or 'select' will
now be NULL delimited, so one has the option of stacks
access via the total count or this new NULL fencepost.
. Input params were simplified and generalized in both
oldproc_open() & close() to enable more than 1 PROCTAB
to be open simultaneously, which was required for get.
. The PROCPS_PIDS_logical_end enum was relocated after
the Item_table making the need to keep it synchronized
more apparent (if the table expands it's right there).
. The 'Public function' section of the source file was
subdivided into 1) the three basic required functions;
and 2) functions that can sometimes vary between APIs.
Signed-off-by: Jim Warner <james.warner@comcast.net>
if (info->flags | PROC_UID)
Something OR a non-zero constant is always true.
Looks like it should be and'ed for the standard flag masking
pattern.
References:
Coverity #99118
Signed-off-by: Craig Small <csmall@dropbear.xyz>
A while loop used n >=0
A branch inside the loop had a test for n<0
That's never going to happen!
So the branch must go. There is similiar code outside
the branch. Yes, I miss pythons while else here.
Also cleaned up the ***-awful indentation this function had.
No wonder that branch lived for so long there.
References:
Coverity #99119
Signed-off-by: Craig Small <csmall@dropbear.xyz>
procps_slabnode_stacks_sort() would check if stacks was NULL
*after* the pointer was derefenced in the initialisation of
the variable p.
p is now assigned after the check.
References:
Coverity #99160
Because of the vast quantities of virtual memory which
may be allocated, it initially seemed like a good idea
to provide for a widest possible range through the use
of a 'ull_int' result type. However, on second thought
the implementation was a bit flawed for these reasons:
. that underlying meminfo_data variable 'VmallocTotal'
is 'unsigned long' not a required 'unsigned long long'
. there wasn't a convenient way to value it since each
variable was set with a strtoul() call, not strtoull()
So this patch will standardize on the 'ul_int' results
type (and reduce the associated delta to 's_int' too).
For now, we'll rely on protections under a 64-bit arch
where a 'ull_int' & 'ul_int' yield identical capacity.
Signed-off-by: Jim Warner <james.warner@comcast.net>
A recent buglet in the <meminfo> interface, where many
delta values could legitimately be negative, suggested
a review of this module where a minus was forced to 0.
As it turns out, there are two delta items that indeed
could be negative. They involve the current processes.
So henceforth, DELTA_PROC_BLOCKED & DELTA_PROC_RUNNING
will now be allowed to go negative. I believe that all
other items can only grow. But, if we find more later,
at least the adaptation approach has been established.
[ this whole business of checking for less than zero ]
[ harks back to an old kernel anomaly where negative ]
[ tics were sometimes experienced. top was affected. ]
Signed-off-by: Jim Warner <james.warner@comcast.net>
When the code for DELTA values (among other stuff) was
stolen from the recently revised <stat> interface, the
concept of ever growing values was propagated too. But
here we must manage both growing and shrinking values.
Thus former protections against a negative delta don't
have any place in this module and are hereby banished.
Signed-off-by: Jim Warner <james.warner@comcast.net>
Thus function returns a pointer, not an int, so if there is
an error return NULL and not -errno.
proc/meminfo.c: In function ‘procps_meminfo_select’:
proc/meminfo.c:994:20: warning: return makes pointer from integer
without a cast [-Wint-conversion]
return -ENOMEM;
References:
commit 407f1b71de
This represents the refinement of this interface after
the <stat> API was redesigned. We now follow a pattern
of 'get' for single item retrieval & 'select' for when
multiple items are desired, with just 1 function call.
And again following the <stat> lead this interface now
provides for delta values encompassing most items. The
reason I went cuckoo nuts with those deltas is because
they are essentially free. At the cost of a little RAM
and just one memcpy there's no other price to be paid.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In the commit referenced below it was acknowledged the
procps_stat_get() function will intentionally restrict
returned values by excluding DELTA items among others.
Upon reflection that decision seemed rather arbitrary,
and so has now been lifted. And, while the initial use
of a DELTA will return 0, subsequent calls to that get
function will now return the difference (providing one
second, at least, has elapsed between any such calls).
Reference(s):
. reference to design decisions
commit abc71a46ad
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit is just my usual tweaking after an initial
submission, once the dust has settled & some more desk
checking was performed. Here are the changes included:
. get function need not check the 'stat_was_read' flag
( first time in save_sec == 0 so branch always taken )
. some table logic appeared outside the proper section
. cleanup_stack's loop made to work like assign_result
. eliminated an include of procps.h from stat.h header
. removed 'ext_numitems' from the stacks_extent struct
. changed the alignment of three prototypes for stat.h
. reorganized the pointers for 'stacks_extents' struct
. improved types indentation in header for readability
. lastly, some really gratuitous changes made to align
- comments within that private 'stacks_alloc' function
Signed-off-by: Jim Warner <james.warner@comcast.net
This commit represents a complete redesign of the stat
interface. Gone are the confusing 8 separate accessors
along with their 2 additional read functions. In their
place we have just 3 accessors, with no read required.
That old interface also suffered an inflexibility with
respect to structures. Now we deal with an unchanging
standard 'result' struct enabling future changes where
the binary interface will no longer need to be broken.
And gone is that former unnecessary typedef, used when
dealing with jiffies. Now the standard C type is used.
Our new API also adds some brand new functionality. If
a caller plans to employ successive 'select' or 'reap'
invocations, then delta values are available (which is
actually only what that top program is interested in).
At some future point a 'sort' function could be easily
introduced to complement the 'reap' function. However,
I saw no need for it at present and so it was omitted.
There were several design decisions which everyone may
not agree with. In support I'll offer these rationals:
. The 'get' function returns a signed long long result
which means a potential loss of some significance. But
I felt the ability to distinguish actual errors (minus
values) from true zero results were worth such a risk.
. The DELTA item enumerators were also made signed and
smaller than their parents. And they are intentionally
grouped as last so as to emphasize those distinctions.
. The SYS type items were excluded from the new 'reap'
function. It would not make sense to duplicate them in
each results stack. They're limited to 'get'/'select'.
. By the same token, some items (DELTA, etc.) will not
be allowed under that 'get' routine. That function was
already open to significant internal overhead (through
subsequent calls like in vmstat.c). That is why it has
been limited via 1 second between reads of /proc/stat.
Lastly, when we finally get around to documenting this
interface there's a real potential toe stubber when it
comes to the numa node portion. The libnuma.so doesn't
really provide any means to retrieve the active nodes.
Thus, any total reported by <stat> is just the highest
node number plus one, as reported by the numa library.
Any unused/inactive nodes are identified through these
. PROCPS_STAT_TIC_ID shows as PROCPS_STAT_NODE_INVALID
By the same token after the STAT_REAP_CPUS_ONLY 'reap'
. PROCPS_STAT_TIC_NUMA_NODE = PROCPS_STAT_NODE_INVALID
Reference(s):
http://www.freelists.org/post/procps/newlib-stat-interface
Signed-off-by: Jim Warner <james.warner@comcast.net>
During work on the new <stat> interface the typedef of
jiffs was eliminated along with the clang warning. The
exact same warning in sysinfo.h now goes bye-bye also.
Reference(s):
./proc/sysinfo.h:16:28: warning: redefinition of typedef 'jiff' is a C11 feature [-Wtypedef-redefinition]
typedef unsigned long long jiff;
Signed-off-by: Jim Warner <james.warner@comcast.net>
The previous commit did not take into account operator evaluation
order so always gave 0. Fixed the macro and added some checks for
the macro.
References:
commit 9abf7d879d
Linux release numbers are x.y.z we assumed but never protected
against x> 0x7fff and y,z > 0xff before.
Linux release in 1991, 1 major release per 6 years so we're fine
until 7452, unless they do way too many minor rels (max being 39)
procps_uptime, previously just plain uptime() used to put the
uptime (as a double) in the first argument and return uptime
(as an int).
It meant if you ran
myuptime2 = procps_uptime(&myuptime1, NULL);
You might get different results for myuptime1 and myuptime2 because
they are different types.
Most library calls use the return value to return the status,
procps_uptime was in the middle.
Until now.
This function will return 0 on success. If you want (for whatever
reason) uptime as an int then cast it.
All of the procps binaries didn't use the return value for uptime
except ps which set a variable to it but never used it anywhere.
While procps_diskstat_ref and procps_diskstat_unref were declared
they actually weren't implemented in the library. Linking some
binary that used these functions gave the following errors:
CCLD vmstat
vmstat.o: In function `disksum_format':
vmstat.c:719: undefined reference to `procps_diskstat_unref'
vmstat.o: In function `diskformat':
vmstat.c:591: undefined reference to `procps_diskstat_unref'
In an attempt to normalize the 'stat' interface, we'll
first shed any reminders of the old readproc interface
by changing file names to be more descriptive & brief.
Signed-off-by: Jim Warner <james.warner@comcast.net>
After the commit referenced below the potential exists
for a SEGV (resulting from an out-of-bounds Item_table
reference if PROCPS_PIDS_physical_end is encountered).
So this patch eliminates that PROCPS_PIDS_physical_end
as no longer necessary and completes the task of using
PROCPS_PIDS_logical_end as a sole necessary fencepost.
Reference(s):
commit e7585992d9
Without breaking either ABI or API just rearrange some
stuff to provide a little more consistent source file.
[ these were prompted by some work on the <stat> API ]
[ in preparation for him borrowing some of our code. ]
Reference(s):
Signed-off-by: Jim Warner <james.warner@comcast.net>
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>
After experimenting with an adaptation of pidof to the
new pids interface, it became apparent that vectorized
versions of those command lines would be necessary. So
this commit adds that option and the strv result type.
And since the stage had been set, a vectorized version
of PROCPS_PIDS_ENVIRON & PROCPS_PIDS_CGROUP was added.
Lastly, any use of 'const' in the result structure was
removed so callers need not be bothered with casts and
compiler warnings. Hopefully, they'll respect a stack.
Signed-off-by: Jim Warner <james.warner@comcast.net>
To ease the transition to the new interface, for other
than that top program, individual read provisions have
been added to the <proc/pids.h> API. This represents a
refinement of a position stated in a post noted below.
Reference(s):
http://www.freelists.org/post/procps/newlib-ps-fix
Signed-off-by: Jim Warner <james.warner@comcast.net>
In my zeal to finalize the initial pids implementation
I omitted some quite important parameter checking from
the above function. Thank goodness top was kind to us.
Also, in anticipation of the additions of single stack
read and supporting functions some items were renamed.
Signed-off-by: Jim Warner <james.warner@comcast.net>
The patch below is where most proc_t fixed size arrays
became simple pointers to char. In that commit changes
to the above function were made so that dynamic memory
was freed which included the program name (cmd) field.
That change was prompted by a valgrind reported memory
leak during development that no longer seems to exist.
However, by keeping the look_up_our_self() changes the
ps command without args then fails to report anything.
So this patch just restores the expected old behavior.
Reference(s):
commit 3881a0844a
Signed-off-by: Jim Warner <james.warner@comcast.net>
It was probably always wrong to have a variable length
proc_t structure. This patch takes all remaining oomem
former suse only options and makes them unconditional.
Signed-off-by: Jim Warner <james.warner@comcast.net>
. traded a complex misaligned memory allocation scheme
in the make_hist function for a simple aligned scheme.
plus memory allocation increases are globally defined.
. changed 1 parameter for procps_pids_stacks_sort() to
better reflect the 'array of pointers', not an address
of a pointer as is used with guys such as 'new/unref'.
. the pids_reap struct was changed slightly to make it
more reflective of it's actual implementation details.
. the Item_table member .mustfree is now .needfree and
that .makehist was now made .needhist for consistency.
. reduced the number of separate 'return NULL;' source
statements in that primary procps_pids_reap() routine.
. ensured consistent reference to sizeof(void *) & not
occasional reference to sizeof(void*) without a space.
. rather than enable/disable validate_stacks via a #if
in the function body, it is now handled via a #define.
. some comments in the procps_pids_reset function were
adjusted to reflect this current implementation. shown
originally, they reflected an aborted attempt to avoid
a testing aberration not fully understood at the time.
. added a summary of the memory overhead cost of HST_t
processing to that UNREF_RPTHASH output at unref time.
. a 'PIDs at max depth:' portion of that UNREF_RPTHASH
enabled #define is now published only when the maximum
depth of hash table entry chains exceed depths of one.
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit is the culmination of efforts to modernize
the library api. It should be treated as a first blush
attempt, especially since I have absolutely no library
design experience. But I did have a very strong desire
to lessen the new library's impact on the top program.
Under this new api, a 'stack' is the equivalent of the
old proc_t. It can be seen as a variable length record
whose contents & order is under complete user control.
That initial stack/record configuration is established
at procps_pids_new() time and will probably serve most
program needs. But, a dynamic & demanding program like
top will later change a stack via procps_pids_reset().
For programs like top & ps, procps_pids_reap() will be
the function that will retrieve all tasks and threads.
Any program that needs to filter / select only certain
processes or users have available other functions that
can be used: procps_pids_stacks_alloc, fill & dealloc.
This implementation attempts to maximize that existing
proven libprocps code base. As we gain more experience
such actual code can be migrated into the pids.c file.
Signed-off-by: Jim Warner <james.warner@comcast.net>
A few minor changes are being made to position the old
readproc logic for a transition to the newlib pid api.
These changes will not impact current users beyond the
the need to recompile such code. Hopefully this should
be very last version change to the deprecated library.
. most char arrays were replaced via char * to dynamic
memory. this was done so that newlib could just assume
ownership of such strings without using a strdup call.
. former user and group name arrays also became char *
but here the reason was because pwcache already cached
those names. so, copying to an array never made sense.
. the concept of QUICK_THREADS used to avoid duplicate
overhead for string data was disabled. it could not be
integrated with the newlib design, at least initially.
. any #define which influenced the size of that proc_t
was disable in the header. it was probably a poor idea
to approach optional features in such a manner anyway.
Signed-off-by: Jim Warner <james.warner@comcast.net>
This commit just corrects the oversight wherein 'item'
was being employed when 'these' was actually intended.
Also, it trades some 'item' use for a more descriptive
input parameter which henceforth is known as a 'dest'.
And, there was one leftover 'next' pointer eliminated.
Finally, some logic was made a tad less dependent upon
enumerator names and a few comments were also updated.
Signed-off-by: Jim Warner <james.warner@comcast.net>
With the new perspective on potential uses of a 'noop'
enumerator (or whatever we decide to call it) there is
no longer a need to provide for any extra 'user' space
in the stack header structures used by slab & meminfo.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In addition to that text shown below the line which is
common to several commit messages, this patch contains
several minor changes with lessor impact upon the API:
. Standard copyright boilerplate was added in .c file.
. The #include header files are ordered alphabetically
now, with all those <sys/??> types separately grouped.
. The header file follows the conventions of indenting
(by 4 spaces) those parameters too lengthy for 1 line.
------------------------------------------------------
. The former 'chains' have now become 'stacks' without
the 'next' pointer in each result struct. The pointers
initially seemed to offer some flexibility with memory
allocations and benefits for the library access logic.
However, user access was always via displacement and a
a statically allocated chain was cumbersome to define.
. An enumerator ending in '_noop' will no longer serve
as a fencepost delimiter. Rather, it has become a much
more important and flexible user oriented tool. Adding
one or more such 'items' in any items list passed into
the library becomes the means of extending the 'stack'
to also include user (not just library) data. Any such
data is guaranteed to never be altered by the library.
. Anticipating PID support, where many different types
must be represented in a result structure, we'll adopt
a common naming standard. And, while not every results
structure currently needs to reflect disparate types a
union will be employed so the same dot qualifier ('.')
can be used consistently when accessing all such data.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In addition to that text shown below the line which is
common to several commit messages, this patch contains
several minor changes with lessor impact upon the API:
. A call to procps_stat_read_jiffs() has been added to
those jiffs functions carrying the 'fill' nomenclature
to parallel like functions in some of our other files.
. The #include header files are ordered alphabetically
now, with all those <sys/??> types separately grouped.
. Standard copyright boilerplate was added in .c file.
. The header file follows the conventions of indenting
(by 4 spaces) those parameters too lengthy for 1 line.
------------------------------------------------------
. The former 'chains' have now become 'stacks' without
the 'next' pointer in each result struct. The pointers
initially seemed to offer some flexibility with memory
allocations and benefits for the library access logic.
However, user access was always via displacement and a
a statically allocated chain was cumbersome to define.
. An enumerator ending in '_noop' will no longer serve
as a fencepost delimiter. Rather, it has become a much
more important and flexible user oriented tool. Adding
one or more such 'items' in any items list passed into
the library becomes the means of extending the 'stack'
to also include user (not just library) data. Any such
data is guaranteed to never be altered by the library.
. Anticipating PID support, where many different types
must be represented in a result structure, we'll adopt
a common naming standard. And, while not every results
structure currently needs to reflect disparate types a
union will be employed so the same dot qualifier ('.')
can be used consistently when accessing all such data.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In addition to that text shown below the line which is
common to several commit messages, this patch contains
several minor changes with lessor impact upon the API:
. A 'read' was added to function procps_slabnode_count
(but only when necessary, i.e. info->nodes_used == 0).
. The #include header files are ordered alphabetically
now, with all those <sys/??> types separately grouped.
------------------------------------------------------
. The former 'chains' have now become 'stacks' without
the 'next' pointer in each result struct. The pointers
initially seemed to offer some flexibility with memory
allocations and benefits for the library access logic.
However, user access was always via displacement and a
a statically allocated chain was cumbersome to define.
. An enumerator ending in '_noop' will no longer serve
as a fencepost delimiter. Rather, it has become a much
more important and flexible user oriented tool. Adding
one or more such 'items' in any items list passed into
the library becomes the means of extending the 'stack'
to also include user (not just library) data. Any such
data is guaranteed to never be altered by the library.
. Anticipating PID support, where many different types
must be represented in a result structure, we'll adopt
a common naming standard. And, while not every results
structure currently needs to reflect disparate types a
union will be employed so the same dot qualifier ('.')
can be used consistently when accessing all such data.
Signed-off-by: Jim Warner <james.warner@comcast.net>
In addition to that text shown below the line which is
common to several commit messages, this patch contains
the following additional change without an API impact:
. The #include header files are ordered alphabetically
now, with all those <sys/??> types separately grouped.
------------------------------------------------------
. The former 'chains' have now become 'stacks' without
the 'next' pointer in each result struct. The pointers
initially seemed to offer some flexibility with memory
allocations and benefits for the library access logic.
However, user access was always via displacement and a
a statically allocated chain was cumbersome to define.
. An enumerator ending in '_noop' will no longer serve
as a fencepost delimiter. Rather, it has become a much
more important and flexible user oriented tool. Adding
one or more such 'items' in any items list passed into
the library becomes the means of extending the 'stack'
to also include user (not just library) data. Any such
data is guaranteed to never be altered by the library.
. Anticipating PID support, where many different types
must be represented in a result structure, we'll adopt
a common naming standard. And, while not every results
structure currently needs to reflect disparate types a
union will be employed so the same dot qualifier ('.')
can be used consistently when accessing all such data.
Signed-off-by: Jim Warner <james.warner@comcast.net>
While meminfo does implement only the singular version
of 'xxxx_chain_fill', it was wrong to limit the 'read'
to the plural version (only) within our new slabs API.
This kind of inconsistency will only spell trouble for
future users of the new libprocps API. So, this commit
will mean that any form of xxxx_chain(s)_fill function
also includes a 'read', whereas xxx_getchain does not.
Reference(s):
commit aab537bc13http://www.freelists.org/post/procps/newlib-interfaces
Signed-off-by: Jim Warner <james.warner@comcast.net>
With a little help from smatch, this commit eliminates
some inappropriate code. Also some programmer comments
were (barely) improved (i hope) in some small measure.
Reference(s):
smatch: 406 procps_meminfo_chain_fill() warn: variable dereferenced before check 'chain' (see line 403)
Signed-off-by: Jim Warner <james.warner@comcast.net>
With this patch, we will be close to an implementation
which will be needed when accommodating tasks/threads.
The following explanation was from an earlier message:
The slabtop requirements are similar to those of PIDs.
One must accommodate the variable number of slab nodes
(PIDs) while also accepting different data (char * and
unsigned long). Furthermore, some generalized means to
sort all that acquired stuff must somehow be provided.
------------------------------------------------------
So this patch expands the API to provide dynamic chain
allocation plus allow sorting of those dynamic chains.
While specific to slab needs (nodes, not global stats)
it is not too early to begin to think of newlib chains
as the opaque replacement for a deprecated old proc_t.
Better yet, any newlib chain is inherently variable in
length, something the old proc_t couldn't claim to be.
Of course, as we get to PIDs we'll want to grow/shrink
chains (easily accomplished with a special item enum).
And we'll want to grow/shrink those **head arrays too.
But these minor details don't seem insurmountable now.
Signed-off-by: Jim Warner <james.warner@comcast.net>