On Apr 22, 2012, at 9:32 PM, Christiano F. Haesbaert wrote:

> On Sun, Apr 22, 2012 at 09:16:57PM +0200, Franco Fichtner wrote:
>> On Apr 22, 2012, at 7:58 PM, Christiano F. Haesbaert wrote:
>>
>>> On Sun, Apr 22, 2012 at 06:36:41PM +0200, Franco Fichtner wrote:
>>>> Just being paranoid... strncmp?
>>>
>>> Why ? It's a terminated string vs a string literal, what do you wanna
>>> use as the third argument: strlen("AuthenticAmd") ? . 100% pointless.
>>
>> I can see your point and yet it is being used in the line below your
change. Do you want to call that author's intent '100% pointless' as well just
for the sake of winning an argument or do you simply not care about the depth
and inherent wisdom of the code base you are working on?
>>
>
> You rush into conclusions, cpu_model is different, he actually wants
> the first 5 bytes to evaluate to "Intel", not the whole string, which
> could be something like:
>
> hw.model=Intel(R) Xeon(R) CPU E31220 @ 3.10GHz

Since my first mail I am talking about the mycpu_model line in the
diff. It's there. I am asking why it differs from your added line.

It's fine when you feel other people do pointless work or point out
pointless things. On the other hand, other people may not like the
level of hostility and resistance to advice (as bad as it may be
in this case). I am no expert on OpenBSD and if this is how tech
discussions are handled, I'm not sure if I ever will.

The intent of your patch is very good, especially with legibility
in mind. But if you touch that line, why do it half-heartedly? And
why ask for comments in the first place?

>>>> And how about consolidating style while at it? "!" vs. "== 0" - see code
bits below change.
>>>
>>> Consolidating how ? Are you suggesting we change all strcmp calls in
>>> kernel to use "== 0" ? Please.
>>
>> Personally, I don't care either way, but it's bad style to ignore the
context and change styles. It makes the code harder to read, understand and
maintain. Take a look. Ok?
>>
>
> You care enough to send an email without even checking the other uses,
> if you did, you'll see that !strcmp is more consistent for this case
> than strncmp.

I care enough to point out an inconsistency in your patch...

>
> *You* are ignoring the context and trying to change styles.

... and now we are talking about me being the evil guy who is trying
to change you and the context. Not gonna happen. :)

>>>>> +    if (!strcmp(cpu_vendor, "AuthenticAMD"))
>>>>>      amd64_errata(ci);
>>>>>
>>>>>  if (strncmp(mycpu_model, "VIA Nano processor", 18) == 0) {


Franco

Reply via email to