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

Octal literals, e.g. 'int i = 010;' are arguably evil #227

Closed
cushon opened this issue Oct 31, 2014 · 5 comments
Closed

Octal literals, e.g. 'int i = 010;' are arguably evil #227

cushon opened this issue Oct 31, 2014 · 5 comments

Comments

@cushon
Copy link
Collaborator

cushon commented Oct 31, 2014

Original issue created by kevinb@google.com on 2014-01-16 at 07:09 PM


I just finished swearing up and down that the integer literal 010 would equal ten, not eight. How embarrassing.

If we have managed to get the 'l' suffix error to work, it might be worth considering a check that forbids all octal literals as well.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by cgdecker@google.com on 2014-01-16 at 09:57 PM


They're kind of useful when converting between numeric POSIX file permissions and other formats, though I admit that's very much an edge case.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2014-03-04 at 12:52 AM


We found many legitimate uses of octal literals for file permissions, and many illegitimate uses for dates. We could possibly special case the checker to allow octal literals that are passed into file-permission-accepting methods.


Status: Accepted
Labels: -Priority-Medium, Priority-Low

@eaftan
Copy link
Contributor

eaftan commented Jan 27, 2015

/cc @kevinb9n
I revived this and wrote a prototype checker. My first pass flagged all octal literals, and of the ~50 instances I looked at, all were correct. The usages fell into 2 buckets:

  1. Specifying months and days in dates, e.g. 'new Date("2014", "10", "01")'. These are OK because they parse as the same value whether they are octal or decimal. The only concern here is if you modify the date to either "08" or "09", you will get a compile error as those are not valid octal values. So you might waste some time understanding the error message, but they are not strictly a correctness issue.

  2. Filesystem permissions, e.g. 'chmod(file, 0755)'. These are also OK because filesystem permissions are octal.

I'm going to update the prototype check to only flag octal values that are not of length 2 or 4. That should weed out these legitimate cases.

@eaftan
Copy link
Contributor

eaftan commented Jan 28, 2015

I special cased my prototype checker to flag octal values that are not of length 2 or 4, to avoid the date and filesystem permissions false positives, but most of the hits were still false positives. I think octal literals may not actually be evil. Closing as will not fix.

@eaftan eaftan closed this as completed Jan 28, 2015
@kevinb9n
Copy link
Contributor

Okay then. Good to know. Thanks for digging into it.

On Tue, Jan 27, 2015 at 9:16 PM, Eddie Aftandilian <notifications@github.com

wrote:

I special cased my prototype checker to flag octal values that are not of
length 2 or 4, to avoid the date and filesystem permissions false
positives, but most of the hits were still false positives. I think octal
literals may not actually be evil. Closing as will not fix.


Reply to this email directly or view it on GitHub
#227 (comment).

Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb@google.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants