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

`three = 1` in the linux sourcecode

github.com

Post reply on HN
Print view View original

41–50 of 83 posts

Re: `three = 1` in the linux sourcecode

#41

Post by thaumasiotes »

rockdoe wrote: post #7
yusukeshinyama wrote: post #6

At line 655: "... In a sparse filesystem it will be the sequence of powers of 3, 5, and 7: ..." So I think this means three = pow(3, 0);

Doesn't explain why it's not 1, 1, 1 or 1, 5, 49.

It does, you're just not paying attention. The comment appears over the function that uses those variables; it's not 1,1,1 because they want to hit 1 only once in the sequence 1,3,5,7,9,25,27,49,81,125, etc.

It doesn't explain why it's not 3,1,7 or 3,5,1, but that's because there is no reason for that.

Re: `three = 1` in the linux sourcecode

#42

Post by _pmf_ »

adobriyan wrote: post #28

Why don't you all "deserves better comment" people send a patch?

> Why don't you all "deserves better comment" people send a patch?

Trying to get a patch into Linux as a new developer without personally knowing one of the (sub-)lieutenants is a futile exercise.

Re: `three = 1` in the linux sourcecode

#43

Post by raverbashing »

And here's how the "rainbows and butterflies" world of "code perfectionists" come crashing down.

Yes, the naming is unfortunate. No, there's probably no way to make it better (and if you think code reviews suck, wait until you see a kernel code review) (Well, this came directly from Linus but there's usually some discussion as well)

But in the end this has shipped and is running. (Well, the double goto from Apple as well, but I doubt that went through a code review)

Re: `three = 1` in the linux sourcecode

#44

Post by stephencanon »

thaumasiotes wrote: post #38
denizozger wrote: post #27

Earlier quoted context omitted.

Agreed, comments are generally a way to compensate failure to express ourselves in the code (in this case bad naming).

Actually, there's a difficulty here that naming can't solve; I don't see a better method than the comment. The goal is to enumerate the powers of 3, 5, and 7, once each. Since power sequences all overlap at x^0 = 1, but we specifically don't want to enumerate 1 three times, we have to give one (or, from an alternative viewpoint, two) of the variables special treatment. Whether you name the variables "three", "five",…

The goal is not to enumerate the powers of 3, 5, and 7. The goal is to iterate through the groups which hold BACKUP superblock/GDT copies. “three”, “five” and “seven” are not especially good names for iterator state, nor is there an obvious good reason for exposing the inner details of iterator state.

Re: `three = 1` in the linux sourcecode

#45

Post by mseebach »

thaumasiotes wrote: post #38
denizozger wrote: post #27

Earlier quoted context omitted.

Agreed, comments are generally a way to compensate failure to express ourselves in the code (in this case bad naming).

Actually, there's a difficulty here that naming can't solve; I don't see a better method than the comment. The goal is to enumerate the powers of 3, 5, and 7, once each. Since power sequences all overlap at x^0 = 1, but we specifically don't want to enumerate 1 three times, we have to give one (or, from an alternative viewpoint, two) of the variables special treatment. Whether you name the variables "three", "five",…

I think the problem is the mixing of concerns: The generation of the sequence (which apparently is a function of whether the filesystem is sparse or not) and whatever it is trying to verify.

Re: `three = 1` in the linux sourcecode

#46

Post by jakobe »

Lot's of people suggesting to add explanatory comments. I don't think that's a good idea, it would be better to change the names of the variables:

    unsigned counter1 = 1;
    unsigned counter2 = 5;
    unsigned counter3 = 7;
or use an array:

    unsigned counter[3] = {1,5,7};
No more confusion. That being said, I doubt that the names of these local variables is actually a real source of confusion and is not a problem that needs to be fixed.

Re: `three = 1` in the linux sourcecode

#47

Post by ah- »

_pmf_ wrote: post #42
adobriyan wrote: post #28

Why don't you all "deserves better comment" people send a patch?

> Why don't you all "deserves better comment" people send a patch? Trying to get a patch into Linux as a new developer without personally knowing one of the (sub-)lieutenants is a futile exercise.

That doesn't match my experience at all.

Find the maintainer of the relevant subsystem, talk to them via mail or IRC about what's bugging you and how to improve it, implement it, send a patch and it will most likely be accepted.

Where I have had difficulties getting something accepted is if your patch might cause problems later or doesn't solve the problem in a way that aligns with the vision of the maintainer. But while that might be annoying as a developer that wants a problem fixed now, I'd say that it's probably for the better of the whole kernel.

Re: `three = 1` in the linux sourcecode

#48

Post by apaprocki »

_pmf_ wrote: post #42
adobriyan wrote: post #28

Why don't you all "deserves better comment" people send a patch?

> Why don't you all "deserves better comment" people send a patch? Trying to get a patch into Linux as a new developer without personally knowing one of the (sub-)lieutenants is a futile exercise.

I did it (and repeated a few times) and I didn't know anyone. I did it by reading about the process, formatting my patch correctly, and sending it to the appropriate list. If it were so difficult, Linux would not be a success. Now, just like other large projects, maintainers might dislike comment-only nit patches, but if it objectively increases readability it might work.

Re: `three = 1` in the linux sourcecode

#49

Post by troels »

h2s wrote: post #8
simias wrote: post #3

Read the comment above ext4_list_backups right above: https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d2... /* * Iterate through the groups which hold BACKUP superblock/GDT copies in an * ext4 filesystem. The counters should be initialized to 1, 5, and 7 before * calling this for the first time. In a sparse filesystem it will be the * sequence of powers of 3, 5, and 7: 1, 3, 5, 7, 9, 25, 27, 49, 81, ... * F…

See this comment: https://news.ycombinator.com/item?id=7296586 Good commenting is no substitute for good naming. For a variable containing the number 1, "three" is a shitty name.

I don't know. Upon seeing a variable named "three", I wouldn't immediately assume it contained the literal number 3, since that would be rather pointless use of a variable. It's an odd name, but I don't think it'll be mistaken for its literal meaning.

Re: `three = 1` in the linux sourcecode

#50

Post by CrystalCuckoo »

Syssiphus wrote: post #18
lifthrasiir wrote: post #4

https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d2... Variables `three`, `five` and `seven` are better described as `next_power_of_three`, `next_power_of_five` and `next_power_of_seven`. Since the `ext4_list_backups` function should iterate through 1 (= 3^0 = 5^0 = 7^0), 3, 5, 7, 3^2, 5^2, 3^3, 7^2, ... and 1 should not repeat three times, the initial value of `next_power_of_three` (or any of others) should…

A perfect example of a missing code-comment.

Not really, the problem lies in its naming. Comments that explain what the variables represent signify that they should have been better named in the first place to avoid any confusion.
Post reply on HN