6 ms·

Read the comment above ext4_list_backups right above: https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d25a55da5623399a2644f/fs/ext4/resize.c#L652 htt

by simias 13y ago

Read the comment above ext4_list_backups right above: https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d25a55da5623399a2644f/fs/ext4/resize.c#L652 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.

h2s 13y ago
See this comment: https://news.ycombinator.com/item?id=7296586 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.
- simias 13y ago
  Okay? Is that really worthy of being submitted to hn however? The function being called is directly above this declaration. It's a static function not used outside of this file. What are the chances that someone would edit this code without understanding what "three" is used for in this context? Pretty slim I wager. It's good that people are auditing the linux source code but if you stumble upon some weird looking code (which again, is not the case here in my opinion) the right way to deal with it is not to post it on hacker news. Contact the maintainer (look at the MAINTAINER file at the root of the kernel) if possible with a patch to fix the issue.
  - orclev 13y ago
    Depends on the intent of submitting it. If his intent was to point out a potential bug in the Linux kernel then yes, it shouldn't have been submitted. On the other hand what has resulted has been a rather interesting debate about the merits of variable naming vs. proper commenting, with some side commentary about what a shitty name something like three and five make for variables.
- troels 13y ago
  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.
  - gus_massa 13y ago
    The LaTeX source code has definitions for the constants \@ne, \tw@, \thr@@. It's a microoptimization that is useful because of the way the TeX/LaTeX parsers work. I don't understand the details, it's beyond my LaTeX wizardry. http://tex.stackexchange.com/questions/9787/ne-tw-thr http://tex.stackexchange.com/questions/9787/ne-tw-thr On the other hand, I think that this kind of definitions are not useful for C.
- thaumasiotes 13y ago
  But the variable doesn't contain the number 1. As is clearly explained in the comment, it's only initialized to 1; it contains any arbitrary power of 3 (I'm assuming "arbitrary" because the variable is passed by address to a function accepting a pointer; as long as that kind of thing is going on, who knows what's inside the variable). And that's true from initialization onwards, as 1 is the 0th power of 3. If I adjust your comment to "for a variable whose only purpose is to be passed to another function as the parameter called 'three', 'three' is a shitty name", would you endorse that sentiment?
aduitsis 13y ago
If I'm not mistaken, the first number (1) is not considered a power of three, hence the initial (seemingly strange) three=1. See http://www.nongnu.org/ext2-doc/ext2.pdf#page15 http://www.nongnu.org/ext2-doc/ext2.pdf#page15 for a slightly more detailed explanation. "The first version of ext2 (revision 0) stores a copy at the start of every block group, along with backups of the group descriptor block(s). Because this can consume a considerable amount of space for large filesystems, later revisions can optionally reduce the number of backup copies by only putting backups in specific groups (this is the sparse superblock feature). The groups chosen are 0, 1 and powers of 3, 5 and 7." (Edit: Yes, I know that 3^0=1, but the wording "0, 1 and powers of 3, 5" does not imply 3^0. Thanks!)
- glomph 13y ago
  1 is the zeroth power of 3
  - aduitsis 13y ago
    Yes, 3^0=1, but the wording "0, 1 and powers of 3, 5" does not imply 3^0
grannyg00se 13y ago
"as long as it's commented correctly there's no issue" How do you know it is commented correctly? Now you have to understand the code and the comment and mentally confirm them to be in sync. You also have to maintain the comment when you make changes to the code and you have to trust that others are going to do the same. It would be better if the code itself was expressive enough so that a comment wasn't needed.

by simias 13y ago

h2s 13y ago

See this comment: https://news.ycombinator.com/item?id=7296586 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.

simias 13y ago
Okay? Is that really worthy of being submitted to hn however? The function being called is directly above this declaration. It's a static function not used outside of this file. What are the chances that someone would edit this code without understanding what "three" is used for in this context? Pretty slim I wager. It's good that people are auditing the linux source code but if you stumble upon some weird looking code (which again, is not the case here in my opinion) the right way to deal with it is not to post it on hacker news. Contact the maintainer (look at the MAINTAINER file at the root of the kernel) if possible with a patch to fix the issue.
- orclev 13y ago
  Depends on the intent of submitting it. If his intent was to point out a potential bug in the Linux kernel then yes, it shouldn't have been submitted. On the other hand what has resulted has been a rather interesting debate about the merits of variable naming vs. proper commenting, with some side commentary about what a shitty name something like three and five make for variables.
troels 13y ago
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.
- gus_massa 13y ago
  The LaTeX source code has definitions for the constants \@ne, \tw@, \thr@@. It's a microoptimization that is useful because of the way the TeX/LaTeX parsers work. I don't understand the details, it's beyond my LaTeX wizardry. http://tex.stackexchange.com/questions/9787/ne-tw-thr http://tex.stackexchange.com/questions/9787/ne-tw-thr On the other hand, I think that this kind of definitions are not useful for C.
thaumasiotes 13y ago
But the variable doesn't contain the number 1. As is clearly explained in the comment, it's only initialized to 1; it contains any arbitrary power of 3 (I'm assuming "arbitrary" because the variable is passed by address to a function accepting a pointer; as long as that kind of thing is going on, who knows what's inside the variable). And that's true from initialization onwards, as 1 is the 0th power of 3. If I adjust your comment to "for a variable whose only purpose is to be passed to another function as the parameter called 'three', 'three' is a shitty name", would you endorse that sentiment?

aduitsis 13y ago

If I'm not mistaken, the first number (1) is not considered a power of three, hence the initial (seemingly strange) three=1. See http://www.nongnu.org/ext2-doc/ext2.pdf#page15 http://www.nongnu.org/ext2-doc/ext2.pdf#page15 for a slightly more detailed explanation. "The first version of ext2 (revision 0) stores a copy at the start of every block group, along with backups of the group descriptor block(s). Because this can consume a considerable amount of space for large filesystems, later revisions can optionally reduce the number of backup copies by only putting backups in specific groups (this is the sparse superblock feature). The groups chosen are 0, 1 and powers of 3, 5 and 7." (Edit: Yes, I know that 3^0=1, but the wording "0, 1 and powers of 3, 5" does not imply 3^0. Thanks!)

glomph 13y ago
1 is the zeroth power of 3
- aduitsis 13y ago
  Yes, 3^0=1, but the wording "0, 1 and powers of 3, 5" does not imply 3^0

grannyg00se 13y ago

"as long as it's commented correctly there's no issue" How do you know it is commented correctly? Now you have to understand the code and the comment and mentally confirm them to be in sync. You also have to maintain the comment when you make changes to the code and you have to trust that others are going to do the same. It would be better if the code itself was expressive enough so that a comment wasn't needed.