Skip to content

Conversation

@domcleal
Copy link

The block that parses /proc/self/mountinfo to find a selinuxfs filesystem would
return results as an array. On Ruby 1.8, interpolating this into a string for
File.exists? when one result was returned worked, while on Ruby 1.9 it
interpolated as ["/sys/fs/selinux"]/enforce so later failed.

This changes the block to return the single result string rather than an array.
This also fixes #11531 where multiple selinuxfs filesystems could be mounted,
as it returns only the first mountpoint.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this ends up not finding the line, it will return an empty array, and we'll end up with the same problem that we had before. In the case where the line is not found, it would be better to return an empty string, to avoid the problem with the array and File.exists?.

This could be more clearly handled if you split the finding of the line from the returning of the line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks. I've updated the branch, however I've returned /selinux as standard instead of an empty string, as it was the standard fallback. By changing the source /proc file used too, we've got much better compatibility (e.g. on RHEL 5).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks! That was better than my suggestion anyways. We'll try to get this merged in soon. :)

The block that parses /proc/self/mountinfo to find a selinuxfs filesystem would
return results as an array.  On Ruby 1.8, interpolating this into a string for
File.exists? when one result was returned worked, while on Ruby 1.9 it
interpolated as ["/sys/fs/selinux"]/enforce so later failed.

This changes the block to return the single result string rather than an array.

This also fixes #11531 where multiple selinuxfs filesystems could be mounted,
as it returns only the first mountpoint.

The /proc file was changed from /proc/self/mountinfo to /proc/self/mounts for
compatibility with Linux 2.6.25 and older.
@domcleal
Copy link
Author

You're right, I got my paths muddled. /proc/mounts is a symlink to self/mounts, but I've updated the branch now to use /proc/self/mounts consistently.

@HAIL9000 HAIL9000 merged commit 10aa3aa into puppetlabs:1.6.x Jul 24, 2012
whopper pushed a commit to whopper/facter that referenced this pull request Mar 18, 2015
…eck-use-exitcode

(maint) Make cppcheck use exitcode
florindragos pushed a commit that referenced this pull request Jun 15, 2020
* (FACT-2289) Add facterversion on solaris.

* (FACT-2289) Add facterversion on sles.

* (FACT-2289) Add facterversion fact on Rhel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants