pilebones / go-udev

Simple udev implementation in Golang
GNU General Public License v3.0
95 stars 28 forks source link

Fixes for minor warnings reported by static analysis tools #8

Closed stolowski closed 6 years ago

stolowski commented 6 years ago

Hey! A few trivial fixes for warnings reported by static analysis tools that we run as part of our build process - these include spellcheck, go vet, gofmt, nakedret and ineffassign (the last one didn't report any problems), for example nakedret reports:

crawler/device.go:111 getEventFromUEventFile naked returns on 27 line function 
netlink/conn.go:40 Connect naked returns on 17 line function 
netlink/conn.go:77 ReadMsg naked returns on 29 line function 
netlink/uevent.go:34 ParseKObjAction naked returns on 8 line function 
netlink/uevent.go:116 ParseUEvent naked returns on 34 line function 
netlink/matcher.go:45 EvaluateAction naked returns on 11 line function 

While these are completly non-critical, I'd appreciate merging them, thank you.

pilebones commented 6 years ago

Hey thanks for these fixes, I appreciate your initiatives and contributions on this project. It is your fourth PR !

Some feedbacks about changes related to "naked returns", the tool used highlight only the final naked return (at the end of the function). In these simple/short cases, I used explicitly "named return values" pattern, and it allows to shorten code and to avoid to declare variable in the function body. This second subtlety (I aware it is not really idiomatic for that) shouldn't used in all cases because it may be the source of errors such shadow variable. But in my opinion, these cases are well suited. I prefer to keep "naked return" statement everywhere in a function or to change function prototype to remove "named return values" pattern to explicitly return the right value everywhere.

For fixes in files below :

stolowski commented 6 years ago

I agree nakedret suggestions are a bit questionable, you're right. I've reverted them. Thanks!

pilebones commented 6 years ago

Perfect, I have squashed these commits into only one and merge it into master. Thanks for modifications.