On 2024-06-10 00:18, David Brownlee wrote:
On Sun, 9 Jun 2024 at 19:37, matthew green <mrg%eterna23.net@localhost> wrote:magic numbers could be named, "vax_cpudata & 0xff" could become a macro (GET_SIE_MICROCODE_VER(x)?)Sometimes the ucode rev is in 0xff (which is accessed sometimes as "& 0xff", "& 0377", and my favourite "% 0377" :), and sometimes the hardware3 rev is in 0xff, and ucode rev in 0xff00. Its common enough that I think it's worth a macro, with a /* usually, not always */ comment
If anyone did "% 0377" that would be a bug. It's a different thing. However, if they did "% 0400" it would be fine, and I would suspect the compiler would reduce that to an AND (or actually a BICL in VAX assembly) in the end anyway.
Actually, looking again I really rather like the approach in
vax/ka780.c:345 - create a bitflag struct for sid
struct ka78x {
unsigned snr:12,
plant:3,
eco:8,
v785:1,
type:8;
};
and then use
aprint_normal(": KA%s, S/N %d(%d), hardware ECO level %d(%d)\n",
cpu_getmodel() + 7, ka78->snr, ka78->plant, ka78->eco >>
4, ka78->eco);
I do agree that using a bitfield is a very reasonable approach for this, and I did the same for the ka86. But looking at the quoted lines above, I really think eco should be split in two parts instead of the shifting. That seems just silly, if that really is two different numbers to the eco. Also, cpu_getmodel() returns a string, and then 7 is added to that pointer. That seems rather horrible coding, and I think that should also be cleaned up a bit. But that's not really a part of your work, so I wouldn't worry about that here.
However, again - I think changing to use bitfields for these kind of things make the code much cleaner.
Johnny
--
Johnny Billquist || "I'm on a bus
|| on a psychedelic trip
email: bqt%softjar.se@localhost || Reading murder books
pdp is alive! || tryin' to stay hip" - B. Idol