Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace IOMainPort/IOMasterPort() and kIOMainPortDefault/kIOMasterPortDefault with NULL #1333

Merged
merged 2 commits into from Jul 30, 2022

Conversation

kyz
Copy link
Contributor

@kyz kyz commented Jul 25, 2022

The code introduced in #1191 needlessly breaks backwards compatibility with macOS 10.15 and macOS 11, when code is compiled on a macOS 12 host.

Code built on macOS 12 results in binaries that fail because there is no _kIOMainPortDefault symbol in macOS 10 or macOS 11.

It is more useful if the code is OS version invariant.

Let's look at the documentation of the function being called, IOServiceGetMatchingServices():

Declaration

  • mainPort: mach_port_t,

Parameters

  • masterPort: The primary port obtained from IOMasterPort(::). Pass kIOMasterPortDefault to look up the default primary port.

(Yes, Apple hastily and only partially renamed this parameter, but it's a key core value of Apple that they needlessly break backwards compatibility)

We do not need to call IOMasterPort() or IOMainPort(). We can supply kIOMasterPortDefault or kIOMainPortDefault and it will get the default port for us.

Furthemore, note the kIOMasterPortDefault / kIOMainPortDefault discussion, which makes it clear that it is merely a synonym for NULL. You can use NULL as an alternative:

Discussion

  • When specifying a primary port to IOKit functions, the NULL argument indicates "use the default". This is a synonym for NULL, if you'd rather use a named constant.

So this PR replaces all calls to either function and constant with the documented and acceptable alternative of passing NULL (cast to mach_port_t) as the first parameter to IOServiceGetMatchingServices()

@Lomanic
Copy link
Collaborator

Lomanic commented Jul 27, 2022

Indeed, here's another project fixing the same deprecation using NULL libusb/hidapi#406

Looks good to me, though I have no way to test this without any darwin host and I'm not sure how much automatic tests cover this code.

@shirou
Copy link
Owner

shirou commented Jul 29, 2022

Thank you for this awesome PR. It works great, and I agree to change NULL, but I got this warning when I build. Is there any idea to resolve this warning? Thank you!

# github.com/shirou/gopsutil/v3/host
smc_darwin.c:75:41: warning: cast to smaller integer type 'mach_port_t' (aka 'unsigned int') from 'void *' [-Wvoid-pointer-to-int-cast]
PASS

@kyz
Copy link
Contributor Author

kyz commented Jul 29, 2022

I don't get that error on an Intel Mac running macOS 10.15, but yes, mach_port_t is an integral type rather than a pointer, so while the documentation says "NULL", it must mean "0". I chose to write NULL to match what the documentation says, but the value of NULL on macOS is 0 so (mach_port_t) NULL could be changed to 0

In order to affirm it works, I made a small test program and ran it on an Intel Mac with macOS 10.15. It compiles and runs the same with this patch, without this patch, and with the patch changed to use 0 instead of (mach_port_t) NULL

package main

import (
    "fmt"
    "github.com/shirou/gopsutil/v3/disk"
    "github.com/shirou/gopsutil/v3/host"
)

func main() {
    fmt.Println(host.SensorsTemperatures())
    fmt.Println(disk.IOCounters())
}
  1. Without the patch:
$ CGO_ENABLED=1 go run test.go
[{"sensorKey":"TA0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TA1P","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TC0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TC0H","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TC0P","temperature":57,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB0T","temperature":38,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB1T","temperature":38,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB2T","temperature":36,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB3T","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0D","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TG0H","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TH0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TM0S","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TM0P","temperature":51,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0H","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TN0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TI0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TI1P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TW0P","temperature":128,"sensorHigh":0,"sensorCritical":0}] <nil>
map[disk0:{"readCount":15334022,"mergedReadCount":0,"writeCount":8780891,"mergedWriteCount":0,"readBytes":131762569216,"writeBytes":148194582528,      "readTime":6944587,"writeTime":5082312,"iopsInProgress":0,"ioTime":12029578,"weightedIO":0,"name":"disk0","serialNumber":"","label":""}] <nil>
  1. With the patch:
$ CGO_ENABLED=1 go run test.go 
[{"sensorKey":"TA0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TA1P","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TC0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TC0H","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TC0P","temperature":57,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB0T","temperature":38,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB1T","temperature":38,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB2T","temperature":36,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB3T","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0D","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TG0H","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TH0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TM0S","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TM0P","temperature":51,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0H","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TN0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TI0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TI1P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TW0P","temperature":128,"sensorHigh":0,"sensorCritical":0}] <nil>
map[disk0:{"readCount":15334582,"mergedReadCount":0,"writeCount":8785087,"mergedWriteCount":0,"readBytes":131768672256,"writeBytes":148253540352,      "readTime":6945228,"writeTime":5084846,"iopsInProgress":0,"ioTime":12030075,"weightedIO":0,"name":"disk0","serialNumber":"","label":""}] <nil>
  1. Wth the patch, changing (mach_port_t) NULL to 0:
$ CGO_ENABLED=1 go run test.go
[{"sensorKey":"TA0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TA1P","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TC0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TC0H","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TC0P","temperature":57,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB0T","temperature":38,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB1T","temperature":38,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TB2T","temperature":36,"sensorHigh":0,"sensorCritical":0}        {"sensorKey":"TB3T","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0D","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TG0H","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TG0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TH0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TM0S","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TM0P","temperature":51,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0H","temperature":0,"sensorHigh":0,"sensorCritical":0}         {"sensorKey":"TN0D","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TN0P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TI0P","temperature":0,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"TI1P","temperature":0,"sensorHigh":0,"sensorCritical":0}          {"sensorKey":"TW0P","temperature":128,"sensorHigh":0,"sensorCritical":0}] <nil>
map[disk0:{"readCount":15338267,"mergedReadCount":0,"writeCount":8787114,"mergedWriteCount":0,"readBytes":131790503936,"writeBytes":148271783936,      "readTime":6946821,"writeTime":5085590,"iopsInProgress":0,"ioTime":12032411,"weightedIO":0,"name":"disk0","serialNumber":"","label":""}] <nil>

@shirou
Copy link
Owner

shirou commented Jul 30, 2022

Sorry, I have not write my environment. I run with M1-Mac macOS 12.0.1 on scaleway.com.

  • Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:24 PDT 2021; root:xnu-8019.41.5~1/RELEASE_ARM64_T8101 arm64
  • Apple clang version 13.0.0 (clang-1300.0.29.3)
  • ProductVersion: 12.0.1
  • BuildVersion: 21A559

I have confirmed changed to 0 like this works without warnings. Awesome!

status = IOServiceGetMatchingServices(0, match, &drives);

So could you update this PR ? I will merge it.

Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants