51–60 of 83 posts
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.
You're totally wrong about "personal" part.
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",…
Earlier quoted context omitted.
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.
The abbreviated quote I was replying to just confuses things by leaving out critical parts.
Earlier quoted context omitted.
A perfect example of a missing code-comment.
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.
localised comments may help but naming things properly would help more (and negate a comment being required at all), actually adopting some better practices would be a better fix going forward.
looking at the called function's comment:
The counters should be initialized to 1, 5, and 7 before * calling this for the first time.
so how about making a function with the same name, ending in '_first' (maybe rename the original _next) which initialises these variables internally before calling the version intended to be called multiple times...?
since the parameters are not well described by their names renaming them would help too... it is not a pointer to a 3 a 5 or a 7. it might not even be pointing at multiples... but current_multiple_of_three etc. would be better. there is some reason why these values are significant in this algorithm (which I do not know) and the best names would capture that.
the code is moderately difficult to read in the function itself - the method of swapping around a local copy of a pointer based on the conditions is not very easy to follow. although makes sense here to avoid excessive code... comments would help make it clearer what is going on.
Earlier quoted context omitted.
> 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 w…
I agree in general. However, the Embedded Linux world suffers from severe fragmentation and duplicate and/or incompatible work in part due to this policy (the greater good). The ARM platform has been consolidated a bit due to the involvement of very large players, but on the PowerPC side, things are pretty dim.
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.
Earlier quoted context omitted.
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.