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

Add net/route parse, also add util lib Pars… #508

Merged
merged 8 commits into from Jun 15, 2023

Conversation

sansna
Copy link
Contributor

@sansna sansna commented Apr 23, 2023

first time to contribute to this repo, might
adds trivial parse of file net/route.
@pgier

…eIpv4FromHexString function - sansna

Signed-off-by: sansna <1185280650@qq.com>
@sansna
Copy link
Contributor Author

sansna commented Apr 23, 2023

passed tests

@@ -110,3 +112,20 @@ func ParseBool(b string) *bool {
}
return &truth
}

// Transforms uint32 to ipv4 representation
func uint322Ipv4(n uint32) string {
Copy link
Member

Choose a reason for hiding this comment

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

let's rename this to uint32ToIPv4.
You can also probably shorten the body to return binary.LittleEndian.PutUint32(net.IP{}, uint32(n)).String()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems not possible to shorten its body like your said way, happy to rename though.

}

// Input: "00000000"
// output: "0.0.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Rename function and convert the comemnt to a proper doc string:

// ParseIPv4FromHexString takes a string in format ....

net_route.go Outdated
}

hexss := make([]string, 0, 3)
hexss = append(hexss, fields[1], fields[2], fields[7])
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to do hexss := []string{fields[1], fields[2], fields[7]} with make()` before. Same below for uss.

In general though, why don't you assing right away e.g routeline.Destination = util.ParseHexUint64(fields[1])? No need to these temp slices here.

Copy link
Contributor Author

@sansna sansna Apr 30, 2023

Choose a reason for hiding this comment

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

i found no singular form of parse in util lib, so i tried to use the plural form, maybe its better to create these method myself

net_route.go Outdated
return nil, err
}
// hex
routeline.Destination = uint32(*hex[0])
Copy link
Member

Choose a reason for hiding this comment

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

Just create routeline here and assing everything, e.g:

routeline := &NetRouteLine{
  Ifrace: ...,
  Destination: ...
...

@discordianfish
Copy link
Member

Congrats to your first contribution! I have a few comments. Lemme know if you need any clarification!

…- sansna

Signed-off-by: sansna <1185280650@qq.com>
@sansna
Copy link
Contributor Author

sansna commented May 3, 2023

any advices, welcome

@discordianfish
Copy link
Member

Look at the linter errors, still need toget fixed.

@sansna
Copy link
Contributor Author

sansna commented May 19, 2023

Look at the linter errors, still need toget fixed.

definitely, recently i got a job, don't have much time, sorry and thanks.

@sansna sansna force-pushed the add-net-route branch 2 times, most recently from 85ac652 to 4425768 Compare May 22, 2023 01:48
Signed-off-by: sansna <1185280650@qq.com>
@sansna
Copy link
Contributor Author

sansna commented May 22, 2023

please check again, thanks for your help

net_route.go Outdated Show resolved Hide resolved
net_route_test.go Outdated Show resolved Hide resolved
net_route.go Outdated Show resolved Hide resolved
sansna and others added 4 commits May 22, 2023 19:06
Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: 三叁 <1185280650@qq.com>
Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: 三叁 <1185280650@qq.com>
Co-authored-by: Ben Kochie <superq@gmail.com>
Signed-off-by: 三叁 <1185280650@qq.com>
Signed-off-by: sansna <1185280650@qq.com>
@SuperQ SuperQ changed the title @Wentao[add-net-route]: added net/route parse, also add util lib Pars… Add net/route parse, also add util lib Pars… May 23, 2023
Signed-off-by: sansna <1185280650@qq.com>
@sansna
Copy link
Contributor Author

sansna commented Jun 10, 2023

thank you for your work, removed these code, plz check again @discordianfish

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Great

@SuperQ SuperQ merged commit 1d6add5 into prometheus:master Jun 15, 2023
8 checks passed
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.

None yet

3 participants