I finally have an answer to the "what's your favorite line of linux source code" question.
This is a great one too: https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d2...
`three = 1` in the linux sourcecode
71–80 of 83 posts
Re: `three = 1` in the linux sourcecode
#72Earlier quoted context omitted.
The function declared earlier is perfectly fine, the comment is sufficient to explain what it's doing, but functions should be understandable simply by reading their source and in that respect the linked function fails. This would be really simple to solve with a simple comment, E.G. // current power of three, init to 3^0 unsigned three = 1; The fact that this looks like a bug at first glance is a pretty good indicat…
This is actually contrary to Linux kernel "good style". Functions should be short and understandable. Comments should be on the top of the function, describing what the function is for. Comments describing variables in functions are discouraged.
Are you supporting that dogma or just stating it?
This brings to mind the Orwell essay on language usage[1]: Break any of these rules sooner than say anything outright barbarous.
IMO having "three = 1" with no immediate context qualifies as barbarous. Yes it would be best to rename the variable, but, failing that, just toss in a freaking comment for common sense's sake.
Re: `three = 1` in the linux sourcecode
#73Earlier quoted context omitted.
The function declared earlier is perfectly fine, the comment is sufficient to explain what it's doing, but functions should be understandable simply by reading their source and in that respect the linked function fails. This would be really simple to solve with a simple comment, E.G. // current power of three, init to 3^0 unsigned three = 1; The fact that this looks like a bug at first glance is a pretty good indicat…
This is actually contrary to Linux kernel "good style". Functions should be short and understandable. Comments should be on the top of the function, describing what the function is for. Comments describing variables in functions are discouraged.
As a general rule that's probably fine, but in cases where a variable is confusing either a better name or a explanatory comment is probably appropriate. Ideally you'd pick a name that fully captures the intent of the variable, but in this case it's rather vague, and a name that isn't would be rather unwieldy, so a comment is a nice compromise. The alternative would be to call it something like:
unsigned current_power_of_three = 1;
Re: `three = 1` in the linux sourcecode
#74Earlier quoted context omitted.
The explanatory comment is 40 lines earlier in the same file where the function those variables are being passed to is defined. It need not be repeated every couple lines; "being clear to outsiders linked to a specific line of a specific file without context" is not a reasonable concern.
The function declared earlier is perfectly fine, the comment is sufficient to explain what it's doing, but functions should be understandable simply by reading their source and in that respect the linked function fails. This would be really simple to solve with a simple comment, E.G. // current power of three, init to 3^0 unsigned three = 1; The fact that this looks like a bug at first glance is a pretty good indicat…
It's C :) edit: It's Linux kernel :)
/* current power of three, init to 3^0 */Re: `three = 1` in the linux sourcecode
#75Earlier quoted context omitted.
The function declared earlier is perfectly fine, the comment is sufficient to explain what it's doing, but functions should be understandable simply by reading their source and in that respect the linked function fails. This would be really simple to solve with a simple comment, E.G. // current power of three, init to 3^0 unsigned three = 1; The fact that this looks like a bug at first glance is a pretty good indicat…
It's C :) edit: It's Linux kernel :) /* current power of three, init to 3^0 */
// became valid C in C99. 15 years ago.
Re: `three = 1` in the linux sourcecode
#76Earlier quoted context omitted.
It's C :) edit: It's Linux kernel :) /* current power of three, init to 3^0 */
// became valid C in C99. 15 years ago.
Sorry, I should have said it's Linux kernel [1].
Linux style for comments is the C89 "/* ... */" style.
Don't use C99-style "// ..." comments.
Re: `three = 1` in the linux sourcecode
#77Lot'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 t…
> or use an array
That's absolutely horrible: you've now introduced the possibility of accidental out-of-bounds access where there previously was none, not to mention that you just forced the compiler to put those variables on the stack instead of using registers.
Contrived example:
int array(int lol)
{
unsigned counter[3] = {1,5,7};
for (int i = 0; i Disassembly of section .text:
0000000000000000 :
0: 85 ff test %edi,%edi
2: c7 44 24 e8 01 00 00 movl $0x1,-0x18(%rsp)
9: 00
a: c7 44 24 ec 05 00 00 movl $0x5,-0x14(%rsp)
11: 00
12: c7 44 24 f0 07 00 00 movl $0x7,-0x10(%rsp)
19: 00
1a: 7e 5f jle 7b
1c: 41 b8 01 00 00 00 mov $0x1,%r8d
22: 31 c9 xor %ecx,%ecx
24: be 56 55 55 55 mov $0x55555556,%esi
29: eb 1f jmp 4a
2b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
30: 89 c8 mov %ecx,%eax
32: f7 ee imul %esi
34: 89 c8 mov %ecx,%eax
36: c1 f8 1f sar $0x1f,%eax
39: 29 c2 sub %eax,%edx
3b: 8d 04 52 lea (%rdx,%rdx,2),%eax
3e: 89 ca mov %ecx,%edx
40: 29 c2 sub %eax,%edx
42: 48 63 c2 movslq %edx,%rax
45: 44 8b 44 84 e8 mov -0x18(%rsp,%rax,4),%r8d
4a: 89 c8 mov %ecx,%eax
4c: f7 ee imul %esi
4e: 89 c8 mov %ecx,%eax
50: c1 f8 1f sar $0x1f,%eax
53: 29 c2 sub %eax,%edx
55: 8d 04 52 lea (%rdx,%rdx,2),%eax
58: 89 ca mov %ecx,%edx
5a: 83 c1 01 add $0x1,%ecx
5d: 29 c2 sub %eax,%edx
5f: 39 f9 cmp %edi,%ecx
61: 48 63 c2 movslq %edx,%rax
64: 41 8d 50 01 lea 0x1(%r8),%edx
68: 89 54 84 e8 mov %edx,-0x18(%rsp,%rax,4)
6c: 75 c2 jne 30
6e: 8b 44 24 ec mov -0x14(%rsp),%eax
72: 03 44 24 e8 add -0x18(%rsp),%eax
76: 03 44 24 f0 add -0x10(%rsp),%eax
7a: c3 retq
7b: b8 0d 00 00 00 mov $0xd,%eax
80: c3 retq
81: 66 66 66 66 66 66 2e data32 data32 data32 data32 data32 nopw %cs:0x0(%rax,%rax,1)
88: 0f 1f 84 00 00 00 00
8f: 00
0000000000000090 :
90: 85 ff test %edi,%edi
92: 7e 52 jle e6
94: 31 c9 xor %ecx,%ecx
96: 41 b8 07 00 00 00 mov $0x7,%r8d
9c: 41 b9 05 00 00 00 mov $0x5,%r9d
a2: 41 bb 01 00 00 00 mov $0x1,%r11d
a8: 41 ba 56 55 55 55 mov $0x55555556,%r10d
ae: 89 c8 mov %ecx,%eax
b0: 89 ce mov %ecx,%esi
b2: 41 f7 ea imul %r10d
b5: c1 fe 1f sar $0x1f,%esi
b8: 89 c8 mov %ecx,%eax
ba: 29 f2 sub %esi,%edx
bc: 8d 14 52 lea (%rdx,%rdx,2),%edx
bf: 29 d0 sub %edx,%eax
c1: 83 f8 01 cmp $0x1,%eax
c4: 74 09 je cf
c6: 83 f8 02 cmp $0x2,%eax
c9: 74 08 je d3
cb: 41 83 c3 01 add $0x1,%r11d
cf: 41 83 c1 01 add $0x1,%r9d
d3: 83 c1 01 add $0x1,%ecx
d6: 41 83 c0 01 add $0x1,%r8d
da: 39 f9 cmp %edi,%ecx
dc: 75 d0 jne ae
de: 45 01 d9 add %r11d,%r9d
e1: 43 8d 04 01 lea (%r9,%r8,1),%eax
e5: c3 retq
e6: b8 0d 00 00 00 mov $0xd,%eax
eb: c3 retq
Re: `three = 1` in the linux sourcecode
#78Earlier quoted context omitted.
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.
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 co…
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.
Re: `three = 1` in the linux sourcecode
#79Earlier quoted context omitted.
The function declared earlier is perfectly fine, the comment is sufficient to explain what it's doing, but functions should be understandable simply by reading their source and in that respect the linked function fails. This would be really simple to solve with a simple comment, E.G. // current power of three, init to 3^0 unsigned three = 1; The fact that this looks like a bug at first glance is a pretty good indicat…
This is actually contrary to Linux kernel "good style". Functions should be short and understandable. Comments should be on the top of the function, describing what the function is for. Comments describing variables in functions are discouraged.
>"Also, try to avoid putting comments inside a function body: if the function is so complex that you need to separately comment parts of it, you should probably go back to chapter 4 for a while. You can make small comments to note or warn about something particularly clever (or ugly), but try to avoid excess. Instead, put the comments at the head of the function, telling people what it does, and possibly WHY it does it."
The code example being discussed has comments sprinkled throughout the functions, and the "good style" / standard also says try to avoid not avoid at all cost. Also, something about blind adherence to rulebooks.
Re: `three = 1` in the linux sourcecode
#80Lot'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 t…
> or use an array That's absolutely horrible: you've now introduced the possibility of accidental out-of-bounds access where there previously was none, not to mention that you just forced the compiler to put those variables on the stack instead of using registers. Contrived example: int array(int lol) { unsigned counter[3] = {1,5,7}; for (int i = 0; i The output is quite different (gcc -c -O3 -std=gnu99): Disassembly…
This is filesystem code. Any miniscule performance gain would be eclipsed by the actual reads/writes. This code should really be -Os'd to save cache.