Skip to content

fix linting issues #325

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.

Sign up for GitHub

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

Jump to bottom
Open
thaJeztah wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix linting issues #325

thaJeztah wants to merge 2 commits into from
+21 −18

Conversation

@thaJeztah
Copy link
Contributor

@thaJeztah thaJeztah commented Jan 10, 2025

@thaJeztah thaJeztah requested a review from a team as a code owner January 10, 2025 23:23
@thaJeztah thaJeztah mentioned this pull request Jan 10, 2025
Open
@thaJeztah
Copy link
Contributor Author

thaJeztah commented Jan 10, 2025

cc @kevpar if you have permissions to kick CI ❤️

@katiewasnothere
Copy link

katiewasnothere commented Jan 15, 2025

@thaJeztah It looks like the CI is still failing

@thaJeztah
Copy link
Contributor Author

thaJeztah commented Jan 15, 2025

Yeah, I noticed; I suspect the other failures may also be with newer go versions, or updated golangci-lint.

I think they're all about integer conversion in generated code; haven't checked if they can be ignored (temporarily) or need fixing;

mode=readonly] in [] ...
  ::high file=wim/lzx/lzx.go,line=137,col=13::G115: integer overflow conversion uint32 -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=191,col=26::G115: integer overflow conversion uint -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=200,col=38::G115: integer overflow conversion int -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=376,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=377,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=378,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
  ::high file=wim/lzx/lzx.go,line=526,col=51::G115: integer overflow conversion int -> uint16 (gosec)

@mat007
Copy link

mat007 commented Jan 30, 2025

I opened a separate PR to fix the «Go Generate» failure: #326

kiashok
kiashok requested changes Jan 30, 2025
View reviewed changes
Copy link

@kiashok kiashok left a comment

Choose a reason for hiding this comment

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

CI is still failing

@kiashok
Copy link

kiashok commented Jan 30, 2025

@mat007 @thaJeztah looks like the CIs are still failing

@mat007
Copy link

mat007 commented Jan 31, 2025 •
edited
Loading

Maybe these should be pinned versions? 🤷

go-winio/.github/workflows/ci.yml

Lines 7 to 9 in bdc6c11

GO_VERSION: "oldstable"
GOTESTSUM_VERSION: "latest"
GOLANGCILINT_VERSION: "latest"

@mat007
Copy link

mat007 commented Jan 31, 2025

There are 104 linting errors remaining, all due to conversions. I’ve looked at a few and they all seem to be similar, for instance

go-winio/pkg/etw/fieldopt.go

Line 165 in bdc6c11

ed.writeUint16(uint16(len(values)))
raises

  ::high file=pkg/etw/fieldopt.go,line=165,col=24::G115: integer overflow conversion int -> uint16 (gosec)

What would be best here? To «nolint» them all?

@mat007 mat007 mentioned this pull request Feb 11, 2025
Open
@thaJeztah
fix linting issues
9f2aadb 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
golangci: temporarily ignore G115 (gosec)
a6d7caf 
It currently produces too many (false) positives in generated code, which
needs to be reviewed. For the time being, exclude this check.

    ::high file=wim/lzx/lzx.go,line=137,col=13::G115: integer overflow conversion uint32 -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=191,col=26::G115: integer overflow conversion uint -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=200,col=38::G115: integer overflow conversion int -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=376,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=377,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=378,col=20::G115: integer overflow conversion uint32 -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=526,col=51::G115: integer overflow conversion int -> uint16 (gosec)
    ::high file=wim/lzx/lzx.go,line=546,col=23::G115: integer overflow conversion int64 -> int32 (gosec)
    ::high file=wim/lzx/lzx.go,line=547,col=16::G115: integer overflow conversion uint32 -> int32 (gosec)
    ::high file=wim/lzx/lzx.go,line=555,col=53::G115: integer overflow conversion int32 -> uint32 (gosec)
    ::high file=wim/lzx/lzx.go,line=567,col=32::G115: integer overflow conversion int -> uint16 (gosec)
    ::high file=internal/stringbuffer/wstring.go,line=131,col=47::G115: integer overflow conversion int -> uint32 (gosec)
    ::high file=pkg/bindfilter/bind_filter.go,line=231,col=66::G115: integer overflow conversion int -> uint32 (gosec)
    ::high file=pkg/bindfilter/bind_filter.go,line=241,col=16::G115: integer overflow conversion int -> uint32 (gosec)
    ::high file=pkg/process/process.go,line=19,col=23::G115: integer overflow conversion int -> uint32 (gosec)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
thaJeztah commented Apr 25, 2025
View reviewed changes
.golangci.yml
Comment on lines +75 to +77
gosec:
excludes:
- G115 # G115: integer overflow conversion; TODO: currently too many (false) positives in generated code that need to be verified
Copy link
Contributor Author

@thaJeztah thaJeztah Apr 25, 2025

Choose a reason for hiding this comment

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

I added a global ignore for this one for now; I think most of these are likely false positives in generated code, but would have to be reviewed by someone with more understanding of those parts of the code.

@thaJeztah
Copy link
Contributor Author

thaJeztah commented Apr 25, 2025

Pushed some changes, but I don't have a windows machine to verify on locally, but if someone could kick CI again ❤️ @kiashok @katiewasnothere

@helsaawy helsaawy assigned helsaawy and unassigned helsaawy Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kiashok kiashok kiashok requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thaJeztah @katiewasnothere @mat007 @kiashok @helsaawy