`three = 1` in the linux sourcecode

#1

Post by shark234 » Tue, Feb 25, 2014, 9:21 AM UTC

`three = 1` in the linux sourcecode
github.com

Re: `three = 1` in the linux sourcecode

#2

Post by zamalek » Tue, Feb 25, 2014, 9:33 AM UTC

Is this because there is some inconsistency in the GDT documentation?

Re: `three = 1` in the linux sourcecode

#3

Post by simias » Tue, Feb 25, 2014, 9:44 AM UTC

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, ...
     * For a non-sparse filesystem it will be every group: 1, 2, 3, 4, ...
     */

I'm not sure what's so noteworthy about that. There's ton of arcane code in any kernel, as long as it's commented correctly there's no issue.

Re: `three = 1` in the linux sourcecode

#4

Post by lifthrasiir » Tue, Feb 25, 2014, 9:45 AM UTC

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 be 1 and those of others should not be 1. The naming is a bit unfortunate (couldn't they be `threes` etc., for example?) but actually makes sense.

Re: `three = 1` in the linux sourcecode

#5

Post by wazoox » Tue, Feb 25, 2014, 9:45 AM UTC

That deserves an explanation in the code comments...

Re: `three = 1` in the linux sourcecode

#6

Post by yusukeshinyama » Tue, Feb 25, 2014, 9:46 AM UTC

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);

Re: `three = 1` in the linux sourcecode

#7

Post by rockdoe » Tue, Feb 25, 2014, 9:52 AM UTC

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.

Re: `three = 1` in the linux sourcecode

#8

Post by h2s » Tue, Feb 25, 2014, 9:53 AM UTC

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.

Re: `three = 1` in the linux sourcecode

#9

Post by cauliturtle » Tue, Feb 25, 2014, 9:53 AM UTC

> There are only two hard things in Computer Science: cache invalidation and naming things.

> -- Phil Karlton

Re: `three = 1` in the linux sourcecode

#10

Post by h2s » Tue, Feb 25, 2014, 9:55 AM UTC

wazoox wrote: post #5

That deserves an explanation in the code comments...

Better still, it deserves to be replaced with a better name. Comments should not be our first port of call when we notice an issue like this in code. Fix the name. If you can't fix the name, you hang your head in shame and then you write a comment.