implement free features

[D3V1LC0D3R]

No description provided.

[D3V1LC0D3R]

wait im stupid i forgot to test whether it works for count = 1 🥴

[D3V1LC0D3R]

i dont know how important that is but our free doesnt throw an error for count is 0

[codecov]

Codecov Report

Attention: Patch coverage is 76.28205% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 45.98%. Comparing base (57d801c) to head (20f2f18).
Report is 17 commits behind head on main.

Files	Patch %	Lines
src/uu/free/src/free.rs	72.44%	5 Missing and 30 partials ⚠️
tests/common/util.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   49.00%   45.98%   -3.03%     
==========================================
  Files          18       18              
  Lines        2216     2279      +63     
  Branches      309      325      +16     
==========================================
- Hits         1086     1048      -38     
+ Misses       1082     1057      -25     
- Partials       48      174     +126     

Flag	Coverage Δ
macos_latest	?
ubuntu_latest	46.63% <77.27%> (+2.66%)	⬆️
windows_latest	0.10% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[D3V1LC0D3R]

i just realized that swap and total have no human output, gotta fix that

[sylvestre]

please add tests in https://github.com/uutils/procps/blob/main/tests/by-util/test_free.rs
thanks

[D3V1LC0D3R]

@sylvestre i added some basic tests but they dont cover combinations such as -th

[D3V1LC0D3R]

@sylvestre will this pr get merged?

[sylvestre]

probably but please be a bit patient :)

[cakebaker]

This test is about -c, so I would omit the -s as it's not necessary.

Suggested change

	let result = new_ucmd!().args(&["-c", "2", "-s", "0"]).succeeds();
	let result = new_ucmd!().args(&["-c", "2"]).succeeds();

[D3V1LC0D3R]

i did that so i have to wait less 😂

[D3V1LC0D3R]

@sylvestre sry xD i was just a bit bored so i cleaned up the code a bit and implemented --lohi, committing after i tested it a bit :)

[cakebaker]

I think there are at least two tests missing: what happens if you pass 0 as a value to -c or -s?

[D3V1LC0D3R]

if -s is 0 the original throws an error for some reason but doesnt for very small values above 0 (that dont get rounded down to 0). if -c is 0 it also throws an error but if -s is defined and -c is undefined it just runs forever.

[D3V1LC0D3R]

i implemented --lohi so that it actually works unlike the original which doesnt (at least free from procps-ng 4.0.4 from archlinux)

[cakebaker]

Why do you think --lohi doesn't work with the original? And your implementation works?

I have my doubts that your implementation works because when I look at your changes, I see no changes made to parse_meminfo(). And the keys HighTotal, HighFree, LowTotal, and LowFree from /proc/meminfo are never read.

[D3V1LC0D3R]

parse_meminfo is called in the match

[sylvestre]

According to code coverage, it seems that a bunch of lines aren't covered by tests. could you please extend the tests ? thanks

[cakebaker]

@D3V1LC0D3R yes, parse_meminfo() is called, though I'm talking about it's implementation. If you look at it, the keys I mentioned above are not handled by this function (nor anywhere else in the program), even though those keys look like they contain the --lohi information, hence my skepticism ;-)

I think it is easy to mistake the output of free --lohi as incorrect. Here is the output from my machine:

               total        used        free      shared  buff/cache   available
Mem:         8024760     3501244      964364      561908     4434544     4523516
Low:         8024760     7060396      964364
High:              0           0           0

Obviously, the "High" values must be incorrect, they must be greater or equal to the "Low" values. However, "Low" and "High" refer to different memory regions and as there is no high memory on my machine (and the aforementioned keys are not in /proc/meminfo), it is shown as 0.

[D3V1LC0D3R]

@cakebaker i thought --lohi was just info about the highest and lowest the memory has been while running free

[D3V1LC0D3R]

@sylvestre anything else?

[sylvestre]

I don't see minmax in my free binary
free --help|grep min
doesn't return anything ?!

[D3V1LC0D3R]

--minmax is a new feature i implemented

[sylvestre]

sorry but for now, we want free to be a dropped in replacement

New features should be done in a separate PR and discussed :)

[D3V1LC0D3R]

so i should remove this feature? ok :(

[sylvestre]

yes, it needs to be discussed first
thanks

[sylvestre]

Code coverage seems to complain that a few lines aren't covered by test, could you please add a few tests? thanks

[D3V1LC0D3R]

@sylvestre i think that is out of date. how can i check that locally? cargo test only tests and doesnt provide any coverage info as far as i know.

[D3V1LC0D3R]

i found the problem. i forgot to remove the test of --minmax

[cakebaker]

I think there is still some misunderstanding about --lohi and thus its implementation is incorrect :|

Here's what I get on my machine:

$ cargo run free --lohi
               total        used        free      shared  buff/cache   available
Mem:         8024716     3797944     1526748      588260     3632572     4226772
Low:               0           0           0
High:              0           0           0
Swap:              0           0           0
$ free --lohi
               total        used        free      shared  buff/cache   available
Mem:         8024716     3774532     1550112      585096     3629456     4250184
Low:         8024716     6474604     1550112
High:              0           0           0
Swap:              0           0           0

In the code, you read HighTotal and the other related keys from /proc/meminfo. That's correct, however, it's possible that those keys are not available in /proc/meminfo, like on my machine. And this case is currently not handled.

[D3V1LC0D3R]

@cakebaker it should be fixed now from what i understand

[D3V1LC0D3R]

i totally forgot about the macos part

[cakebaker]

Thanks for your PR :)