Skip to content

Conversation

gaul
Copy link
Member

@gaul gaul commented Jul 13, 2017

Previously the fopen write mode caused the call to spuriously report
newer sensors. Now we check the newer sensors path explicitly. Also
plug file descriptor leak and more carefully check return values.

@gaul gaul force-pushed the 3.13 branch 4 times, most recently from ba2f399 to 374c070 Compare July 13, 2017 03:18
@dgraziotin
Copy link
Collaborator

This fails on my machine (MacBook Pro 13" 2015, Ubuntu 16.10, Linux 4.8.0-22-generic),

$ sudo bin/mbpfan -f -v
Writing a new .pid file with value 13142 at: /var/run/mbpfan.pid
mbpfan[13142]: Writing a new .pid file with value 13142 at: /var/run/mbpfan.pid
Successfully written a new .pid file with value 13142 at: /var/run/mbpfan.pid
mbpfan[13142]: Successfully written a new .pid file with value 13142 at: /var/run/mbpfan.pid
Using legacy sensor path for kernel < 3.15.0
Found 0 sensors
mbpfan[13142]: mbpfan could not detect any temp sensor. Please contact the developer.
mbpfan could not detect any temp sensor. Please contact the developer.

@gaul
Copy link
Member Author

gaul commented Jul 20, 2017

Perhaps mbpfan needs to test for legacy kernels some other way? Can you share your output from the following command:

$ find /sys/devices/platform/coretemp.0/hwmon
/sys/devices/platform/coretemp.0/hwmon
/sys/devices/platform/coretemp.0/hwmon/hwmon0
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/control
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/async
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_enabled
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_active_kids
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_active_time
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/autosuspend_delay_ms
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_status
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_usage
/sys/devices/platform/coretemp.0/hwmon/hwmon0/power/runtime_suspended_time
/sys/devices/platform/coretemp.0/hwmon/hwmon0/device
/sys/devices/platform/coretemp.0/hwmon/hwmon0/subsystem
/sys/devices/platform/coretemp.0/hwmon/hwmon0/uevent

I will break out the double-free change into a separate commit.

@gaul gaul force-pushed the 3.13 branch 2 times, most recently from 39b57b8 to dac0874 Compare August 29, 2017 04:47
@gaul gaul changed the title Correctly test directory and prevent invalid frees Correctly test directory on Linux 3.13 Aug 29, 2017
@gaul
Copy link
Member Author

gaul commented Sep 30, 2017

If anyone running a newer kernel can provide this information it would help me complete this pull request.

@dgraziotin
Copy link
Collaborator

dgraziotin commented Oct 1, 2017 via email

@dgraziotin
Copy link
Collaborator

@gaul here is the output (iMac16,2, 4.8.0-22-generic)

$ find /sys/devices/platform/coretemp.0/hwmon
/sys/devices/platform/coretemp.0/hwmon
/sys/devices/platform/coretemp.0/hwmon/hwmon1
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp5_crit
/sys/devices/platform/coretemp.0/hwmon/hwmon1/device
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp3_crit_alarm
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp4_crit
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp1_max
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp5_max
/sys/devices/platform/coretemp.0/hwmon/hwmon1/subsystem
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp3_label
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp2_input
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp3_crit
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp5_input
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_active_kids
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_suspended_time
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/autosuspend_delay_ms
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_enabled
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_active_time
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/control
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/async
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_usage
/sys/devices/platform/coretemp.0/hwmon/hwmon1/power/runtime_status
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp4_crit_alarm
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp2_crit
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp2_max
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp5_crit_alarm
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp1_label
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp4_label
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp1_crit
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp3_input
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp3_max
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp1_crit_alarm
/sys/devices/platform/coretemp.0/hwmon/hwmon1/uevent
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp2_label
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp1_input
/sys/devices/platform/coretemp.0/hwmon/hwmon1/name
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp2_crit_alarm
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp5_label
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp4_input
/sys/devices/platform/coretemp.0/hwmon/hwmon1/temp4_max

mbpfan-master works correctly here (and works on my MacBook Pro)

@gaul
Copy link
Member Author

gaul commented Oct 4, 2017

@dgraziotin Would you mind testing the updated patch?

@gaul gaul changed the title Correctly test directory on Linux 3.13 Correctly test directory on Linux prior to 3.15 Oct 4, 2017
Previously the fopen write mode caused the call to spuriously report
newer sensors.  Now we check the newer sensors path explicitly.  Also
plug file descriptor leak and more carefully check return values.
@gaul
Copy link
Member Author

gaul commented Apr 9, 2018

@dgraziotin I hope to upgrade my MacBook soon so I will no longer be able to test this pull request. Could you review it so that I can test any modifications?

@gaul gaul merged commit 885a084 into linux-on-mac:master Aug 20, 2018
@gaul gaul deleted the 3.13 branch August 20, 2018 22:12
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