On 12/26/2010 06:29 AM, ehem+grub@m5p.com wrote:
>
>>> stage. Second, perhaps minor, but I'm surprised someone chose to break
>>> the checksum verification code in grub-core/partmap/sun.c and didn't
>>> didn't keep the magic number check with it.
>>>       
>   
>> This seems to be just moving the code around. There are many different
>> coding styles. Changing from one of them to another is waste of effort
>> unless it's done to uniformise with rest of codebase. I don't have
>> strong preference to one or another style so I prefer not to touch anything.
>>     
> Leaving it alone for now is fine by me. I just found it rather strange to
> see two distinct styles in the exact same bit of code. I'd expect either
> all of the verification or none of the verification broken into a
> distinct function.
>
>   
checking magic number is a simple check (a == b) whereas checksum check
is a composite (you need to compute checksum first) so moving checksum
out in a separate function makes sense
> I will in fact be implementing breaking detection functions into distinct
> functions uniformly,
I'm not fond of "let's change it just because we can". There are much
more real tasks on the project. Come to #grub and I'm sure will find
something for you.
>  because there is a deeper issue lurking here. Looks
> to be pure luck no one ran into an unpleasant bug lurking in the existing
> design.
>
>   
Please show me the real problem then.
> (I can readily provide an illustration of the lurking landmine, later
> though)
>
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko