Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

`three = 1` in the Linux sourcecode (2014)

github.com

Post reply on HN
Print view View original

21–30 of 105 posts

Re: `three = 1` in the Linux sourcecode (2014)

#21

Post by derfitt »

mnw21cam wrote: post #20
derfitt wrote: post #15

Earlier quoted context omitted.

'Without digging deeper' isn't really what matters here. A better question would be 'after some very cursory further digging, are we worried?' The first thing I ask myself when I see something that looks as wrong as this does is "Is it possible for this piece of code to be completely broken, and for no one to have noticed yet?" This is really a question about two processes - the one where people look at, read, edit a…

The problem with this argument is that this particular snippet of code doesn't get run very often. It's the bit that works out where the superblock (and all its backups) are stored. Now, normally, only the one copy of the superblock is ever read - the backups are only read if the first copy of it is corrupted in some way. So almost all the time, the results of this code are not used. So if it were broken, then yes it…

That's only a problem with one of the arguments, and is exactly why there are two approaches. Some errors are hard to detect by reading or working on the code, but easier to detect from behavior. Some are the other way around.

In this case, I do not agree that this could have gone unnoticed for a long time, even only looking at the behavior of the running system. When multiplied by the number of installations and how much filesystems are used, it is run a lot. It's also the kind of thing people investigate when it happens in certain settings - we lost a block and our whole filesystem was corrupted. And the kind of thing people test for.

Re: `three = 1` in the Linux sourcecode (2014)

#22

Post by ferdek »

ant6n wrote: post #16
GuB-42 wrote: post #6

Another fun one is ONE = 256 (or another power of two). Intended for fixed point arithmetic where 1 actually means 1/256.

This is nicer example, since the posted one should really have different variable names (e.g. pow3). On the other hand, if this declaration of ONE is global (not contained within some context like a type), then perhaps it should be something like ONE_F8.

Something like ONE_FIX_UQ8? https://en.wikipedia.org/wiki/Q_%28number_format%29

Re: `three = 1` in the Linux sourcecode (2014)

#23

Post by gumby »

ronyeh wrote: post #11
hinoki wrote: post #3

Earlier quoted context omitted.

Yeah, maybe calling them pow3, pow5, pow7 would be a bit clearer? But a 10s search for a comment cleared it up, so I don’t really see the problem...

I see a problem: https://github.com/torvalds/linux/blob/master/fs/ext4/resize... Upon a quick code review, these lines look buggy: unsigned three = 1; unsigned five = 5; unsigned seven = 7; Without digging deeper, the reader of this code thinks "The first line surely must be a bug, right???"

I agree. The relevant comment is far away.

It’s a local variable so it seems that there would be little impact in renaming it (and the other two). But if the local (kernel in this case) coding conventions discourage variable renaming, a brief comment referring to the later comment would be worthwhile.

Over commenting is a problem, but this would a good use case if renaming is not appropriate.

Re: `three = 1` in the Linux sourcecode (2014)

#24

Post by gumby »

derfitt wrote: post #15
ronyeh wrote: post #11

Earlier quoted context omitted.

I see a problem: https://github.com/torvalds/linux/blob/master/fs/ext4/resize... Upon a quick code review, these lines look buggy: unsigned three = 1; unsigned five = 5; unsigned seven = 7; Without digging deeper, the reader of this code thinks "The first line surely must be a bug, right???"

'Without digging deeper' isn't really what matters here. A better question would be 'after some very cursory further digging, are we worried?' The first thing I ask myself when I see something that looks as wrong as this does is "Is it possible for this piece of code to be completely broken, and for no one to have noticed yet?" This is really a question about two processes - the one where people look at, read, edit a…

> The first thing I ask myself when I see something that looks as wrong as this does is "Is it possible for this piece of code to be completely broken, and for no one to have noticed yet?"

Good reflex. In my first job out of school (big research lab), we would occasionally find odd bugs in the code base (as would be expected...bugs happen). The head of the project would often wonder, “how could the system ever have functioned with this bug? Someone must have deliberately tampered with the source to discredit us.”

The first few times I thought he was joking, but no: he seriously believed this and even tried to investigate.

This was years before source control, which would have answered the question.

Edit: fix iPad-caused typos

Re: `three = 1` in the Linux sourcecode (2014)

#26

Post by kortex »

ronyeh wrote: post #11
hinoki wrote: post #3

Earlier quoted context omitted.

Yeah, maybe calling them pow3, pow5, pow7 would be a bit clearer? But a 10s search for a comment cleared it up, so I don’t really see the problem...

I see a problem: https://github.com/torvalds/linux/blob/master/fs/ext4/resize... Upon a quick code review, these lines look buggy: unsigned three = 1; unsigned five = 5; unsigned seven = 7; Without digging deeper, the reader of this code thinks "The first line surely must be a bug, right???"

    unsigned three = 1;
To me, that is a 20 foot tall neon orange flashing Chesterton's fence. I think, "surely there is a very strong reason for this." Maybe not a good reason, but definitely a compelling one.

Re: `three = 1` in the Linux sourcecode (2014)

#27

Post by scotty79 »

oji0hub wrote: post #13
scotty79 wrote: post #12

Earlier quoted context omitted.

Not really. Nobody sane would think other sane developer with any expeirience would call a simple variable according to what it initially contains. You'd expect either name related to what is the meaning of the contents, or meaningless name.

People make typos and simple mistakes all the time, regardless of experience.

True. But the intention of writing

    int three = 3;
is insane.

I would never ascribe to previous programmer (unless he just learned what variables are) the intention of creating variable (not even constant, which wouldn't make it any better) named 'three' and assigning actual value 3 to it.

Re: `three = 1` in the Linux sourcecode (2014)

#29

Post by ben_w »

kortex wrote: post #26
ronyeh wrote: post #11

Earlier quoted context omitted.

I see a problem: https://github.com/torvalds/linux/blob/master/fs/ext4/resize... Upon a quick code review, these lines look buggy: unsigned three = 1; unsigned five = 5; unsigned seven = 7; Without digging deeper, the reader of this code thinks "The first line surely must be a bug, right???"

unsigned three = 1; To me, that is a 20 foot tall neon orange flashing Chesterton's fence. I think, "surely there is a very strong reason for this." Maybe not a good reason, but definitely a compelling one.

If I was code reviewing this, I would almost certainly insist on a different name to make the WTF go away. I’m not going to suggest one here despite having read the other code comment, as doing so would be bikeshedding — not my codebase and I have no skin in this — the only point I will insist on is that I cannot believe that “three” is a sensible choice of name for whatever it does. Not even what the comment implies it does.

Re: `three = 1` in the Linux sourcecode (2014)

#30

Post by forgottenlogin9 »

scotty79 wrote: post #27
oji0hub wrote: post #13

Earlier quoted context omitted.

People make typos and simple mistakes all the time, regardless of experience.

True. But the intention of writing int three = 3; is insane. I would never ascribe to previous programmer (unless he just learned what variables are) the intention of creating variable (not even constant, which wouldn't make it any better) named 'three' and assigning actual value 3 to it.

If that is insane, you may not have issue with this specific line but you surely have issue with the 2 “insane” lines that follow it?
Post reply on HN