Re: amd64: Check cpu_vendor instead of using CPUID.
Just being paranoid... strncmp? And how about consolidating style while at it?
! vs. == 0 - see code bits below change.
Franco
On 22.04.2012, at 15:12, Christiano F. Haesbaert haesba...@openbsd.org
wrote:
There's no need for doing that somewhat strange comparison, the rest
of the code already uses cpu_vendor.
ok ?
Index: identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.35
diff -d -u -p -r1.35 identcpu.c
--- identcpu.c27 Mar 2012 02:23:04 -1.35
+++ identcpu.c22 Apr 2012 12:59:10 -
@@ -302,7 +302,6 @@ identifycpu(struct cpu_info *ci)
u_int64_t last_tsc;
u_int32_t dummy, val, pnfeatset;
u_int32_t brand[12];
-u_int32_t vendor[4];
char mycpu_model[48];
int i, max;
char *brandstr_from, *brandstr_to;
@@ -433,11 +432,7 @@ identifycpu(struct cpu_info *ci)
#endif
-vendor[3] = 0;
-CPUID(0, dummy, vendor[0], vendor[2], vendor[1]);/* yup, 0 2 1 */
-/* AuthenticAMD:h t u Ai t n e */
-if (vendor[0] == 0x68747541 vendor[1] == 0x69746e65
-vendor[2] == 0x444d4163)/* DMAc */
+if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
Re: amd64: Check cpu_vendor instead of using CPUID.
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.
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.
Franco
On 22.04.2012, at 15:12, Christiano F. Haesbaert haesba...@openbsd.org
wrote:
There's no need for doing that somewhat strange comparison, the rest
of the code already uses cpu_vendor.
ok ?
Index: identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.35
diff -d -u -p -r1.35 identcpu.c
--- identcpu.c27 Mar 2012 02:23:04 -1.35
+++ identcpu.c22 Apr 2012 12:59:10 -
@@ -302,7 +302,6 @@ identifycpu(struct cpu_info *ci)
u_int64_t last_tsc;
u_int32_t dummy, val, pnfeatset;
u_int32_t brand[12];
-u_int32_t vendor[4];
char mycpu_model[48];
int i, max;
char *brandstr_from, *brandstr_to;
@@ -433,11 +432,7 @@ identifycpu(struct cpu_info *ci)
#endif
-vendor[3] = 0;
-CPUID(0, dummy, vendor[0], vendor[2], vendor[1]);/* yup, 0 2 1 */
-/* AuthenticAMD:h t u Ai t n e */
-if (vendor[0] == 0x68747541 vendor[1] == 0x69746e65
-vendor[2] == 0x444d4163)/* DMAc */
+if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
Re: amd64: Check cpu_vendor instead of using CPUID.
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?
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?
+if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
Franco
Re: amd64: Check cpu_vendor instead of using CPUID.
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
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.
*You* are ignoring the context and trying to change styles.
+if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
Franco
Re: amd64: Check cpu_vendor instead of using CPUID.
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
