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

[net] Implements windows net package Connections and ConnectionsPid #549

Merged
merged 2 commits into from
Sep 1, 2018

Conversation

pytimer
Copy link
Contributor

@pytimer pytimer commented Jul 9, 2018

Sorry, i am busy in recent days.

Implement #534 . Support for Windows net package, i implements Connections() and ConnectionsPid() two functions.

Usage is similar with linux net:

package main

import (
	"fmt"

	"github.com/shirou/gopsutil/net"
)

func main() {
	fmt.Println(net.Connections("tcp"))
	fmt.Println(net.ConnectionsPid("tcp4", 8092))
}

I don't know other functions how to implement, if anyone have idea, welcome to tell me. Thanks.

@Lomanic
Copy link
Collaborator

Lomanic commented Jul 9, 2018

Thanks for this great PR @pytimer.

I won't be able to test this before the weekend, but just reading the code:

  • types_platform.go files are used for cgo/go-tool -godefs, you should merge types_windows.go inside net_windows.go
  • merge net_windows_dll.go inside net_windows.go, it would be the only file with this name (pkg_platform_dll.go) otherwise
  • don't export new functions only defined on a given platform (Windows) and not in the other ones (GetExtendedTcpTable, GetExtendedUdpTable)
  • TCPStatuses is not defined the same way than in Linux (different types even), hence I think 3rd-party code using this variable would not be not portable between these platforms.

@pytimer
Copy link
Contributor Author

pytimer commented Jul 9, 2018

Thanks for your reply.

I think i should do at least two things, as follows:

  • merge net_windows_dll.go and types_windows.go inside net_windows.go, but i am afried this file too many lines.

  • update functions not exposed.

i don't understand TCPStatuses, can you explain it again? thanks.

@pytimer
Copy link
Contributor Author

pytimer commented Jul 9, 2018

@Lomanic i merge net_windows_dll.go and types_windows.go inside net_windows.go, i update TCPStatuses to tcpStatuses.

I don't know i understand right? If it wrong, welcome to point out. Thanks.

@Lomanic
Copy link
Collaborator

Lomanic commented Jul 9, 2018

TCPStatuses is already defined on Linux in the same package with an incompatible type here, this is what I'm talking about. @shirou implemented this in 32c62b5 so he is the most knowledgeable about that and about interoperability between these platforms.

@pytimer
Copy link
Contributor Author

pytimer commented Jul 10, 2018

I think can we move common variable to net.go? like kindTCP4, etc..

Windows net status is different from linux, so i defined new variable.This variable now is private, and itonly used in net package, not used by 3rd-party code.

is it deal right?

@shirou
Copy link
Owner

shirou commented Jul 13, 2018

Thank you for your very cool contribution!

TCPStatuses is used to parse TCP status for linux proc only. So we don't need to use for other platforms. (It is exported but when I wrote this, I didn't understand well the exposure is important)

@pytimer
Copy link
Contributor Author

pytimer commented Jul 13, 2018

I already change net_windows.go TCPStatuses to tcpStatuses, it used to parse tcp status for windows.

@Lomanic
Copy link
Collaborator

Lomanic commented Jul 14, 2018

Don't export the following types/constants and make them Go-compliant (convert ALL_CAPS to mixedCaps, put a comment referencing their proper name in the win32 API docs, check with golint, see also Effective Go)

  • MIB_TCP_STATE
  • TCP_TABLE_CLASS
  • TCP_TABLE_BASIC_LISTENER
  • TCP_TABLE_BASIC_CONNECTIONS
  • TCP_TABLE_BASIC_ALL
  • TCP_TABLE_OWNER_PID_LISTENER
  • TCP_TABLE_OWNER_MODULE_CONNECTIONS
  • TCP_TABLE_OWNER_MODULE_ALL
  • UDP_TABLE_CLASS
  • UDP_TABLE_BASIC
  • UDP_TABLE_OWNER_PID
  • UDP_TABLE_OWNER_MODULE
  • MIB_TCPROW_OWNER_PID
  • MIB_TCPTABLE_OWNER_PID
  • MIB_TCP6ROW_OWNER_PID
  • MIB_TCP6TABLE_OWNER_PID
  • PMIB_TCPTABLE_OWNER_PID_ALL
  • PMIB_TCP6TABLE_OWNER_PID_ALL
  • MIB_UDPROW_OWNER_PID
  • MIB_UDPTABLE_OWNER_PID
  • MIB_UDP6ROW_OWNER_PID
  • MIB_UDP6TABLE_OWNER_PID
  • PMIB_UDPTABLE_OWNER_PID
  • PMIB_UDP6TABLE_OWNER_PID

Otherwise I tested it on Windows, excellent work @pytimer, it works great!

@pytimer
Copy link
Contributor Author

pytimer commented Jul 15, 2018

ok. I will see about you said. Thanks.

@pytimer
Copy link
Contributor Author

pytimer commented Jul 21, 2018

@Lomanic I send a new commit to fix this PR problem, you can check it again.

If something missing, please tell me. Thanks.

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.

3 participants