Skip to content

Feat/expose process info#6

Open
halfcrazy wants to merge 2 commits into
cakturk:masterfrom
halfcrazy:feat/expose-process-info
Open

Feat/expose process info#6
halfcrazy wants to merge 2 commits into
cakturk:masterfrom
halfcrazy:feat/expose-process-info

Conversation

@halfcrazy

Copy link
Copy Markdown

No description provided.

@cakturk cakturk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

BTW, it would be better to introduce go module support in a separate commit.

Comment thread netstat/netstat.go
name string
Pid int
Name string
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It makes sense, a few people already asked me to export these fields.

Comment thread netstat/netstat_linux.go
if err != nil {
return nil, err
}
binary.LittleEndian.PutUint32(ip[i:j], uint32(u))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good catch.

Comment thread netstat/netstat_linux.go
}
n, err := stat.Read(buf[:])
stat.Close()
if err != nil {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is wrong. You don't need to check for nil there. os.Open always returns non-nil
value if it succeeded. Check the source code for os.Open.

Comment thread netstat/netstat_linux.go
}
if f != nil {
defer f.Close()
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Here, again, the same situation as above. Nil-check is unnecessary here as the
os.Open already succeeded.

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