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

draft: syscall support #101

Closed
wants to merge 1 commit into from
Closed

Conversation

ccoVeille
Copy link

I recently found out these constants

They are used, but not so much
https://github.com/search?q=language%3Ago+%22syscall.S_IR%22&type=code

So I made this addition.

Please note, the implementation is naive

I should check against the list bits instead of hard-coding all the values, but it's enough to present the idea

I know I have to manage *os.File, the changes are pretty similar and should be easy.

So please tell me if you find any interest in this

@ccoVeille
Copy link
Author

ccoVeille commented Jan 8, 2025

Thanks for @nobe4 for letting me know these constants existed

Please note this could provide a solution for mnd that reports these numerical filemode as magical numbers.

Here someone is complaining about the fact these are not real "magical numbers"

Having usestdlibvars that suggests a solution seems to be the right direction.

Please note, I don't think the solution should be in mnd, but in your linter, that's why I opened a PR here

@sashamelentyev
Copy link
Owner

@ccoVeille thank you!

@ldez hello, what you mean about this?

@ldez
Copy link
Contributor

ldez commented Jan 8, 2025

the syscall package should be avoided, from https://pkg.go.dev/syscall:

NOTE: Most of the functions, types, and constants defined in this package are also available in the golang.org/x/sys package. That package has more system call support than this one, and most new code should prefer that package where possible. See https://golang.org/s/go1.4-syscall for more information.

The right way is to use os.xxx constants or /x/sys module.

@ccoVeille
Copy link
Author

@ldez
Copy link
Contributor

ldez commented Jan 8, 2025

this linter is about std lib 😉

@ccoVeille
Copy link
Author

But then it's no longer about std lib

I'm unsure

What do you guys think?

@ldez
Copy link
Contributor

ldez commented Jan 8, 2025

I think this is off-topic for this linter.

Also, the meaning behind this linter is to improve readability and maintenance, I am not sure this is a good idea to replace those numbers with constants: those numbers are "easier" to read and use than the constants.

@ccoVeille
Copy link
Author

I wasn't planning it to be enabled often.

I expected some people to look for this notation.

But now, the syscall package is discouraged, and golang/sys is.

It would be out of std lib constants, so out of this linter.

Things add them up badly.

I'm likely to close this PR

@ccoVeille ccoVeille mentioned this pull request Jan 8, 2025
@ccoVeille ccoVeille closed this Jan 8, 2025
@ccoVeille
Copy link
Author

I closed @sashamelentyev

Feel free to provide a feedback or reopen if you think there is an interest

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