SELinuxProject / selinux-testsuite

This is the upstream SELinux testsuite which is designed as a basic set of regression tests for the SELinux kernel functionality.
GNU General Public License v2.0
53 stars 43 forks source link

RFE: simplify kernel version comparison #56

Closed WOnder93 closed 5 years ago

WOnder93 commented 5 years ago

Replace the clumsy hand-coded script with a dead simple one based on sort -V. It turns out all the users of kvercmp really just check if the running kernel version is greater than or equal to some given version (or the inverse), so let's name the new script 'kver_ge' and make it check exactly that (and just indicate the result via exit code).

jstancek commented 5 years ago

Author of the clumsy script here. Is there a minimum distro version that selinux-testsuite aims to support?

recent sort -V appears to do the right thing (implementation changed over time: strverscmp -> filevercmp). Some old distros (e.g. RHEL5, EOL planned next year) lack the option:

{ echo "2.6.18"; echo "3.10.0"; } | sort -V
./sort: invalid option -- 'V'
Try `./sort --help' for more information.
WOnder93 commented 5 years ago

That's a good point, but I don't think we need to care about anything as old as RHEL-5, at least here in upstream. On such an old system you can simply replace kver_ge with false and it will effectively return the correct result every time ;)

pcmoore commented 5 years ago

Just to be clear @WOnder93, does this fix a problem you are seeing? If not, I'm tempted to leave kvercmp as-is; I don't view it as a problem.

WOnder93 commented 5 years ago

The current kvercmp script doesn't deal well with non-numeric characters in the version string (try e.g. ./kvercmp 5.1-blah 5.1.2), but I'm not sure if that is considered a serious issue. Feel free to close the PR if you don't want to do this change. It's just something I noticed could be done in a simpler and somewhat more robust way.

pcmoore commented 5 years ago

The current kvercmp script doesn't deal well with non-numeric characters in the version string (try e.g. ./kvercmp 5.1-blah 5.1.2), but I'm not sure if that is considered a serious issue.

At the moment I'm thinking we just need to concern ourselves with X.Y.Z version numbers so I'm inclined to ignore the non-numeric values right now. I'm going to close this out, but if this becomes a real issue please go ahead and reopen it.