Re: agp_nvidia port for review

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Mon, Jun 08, 2026 at 11:24:17 +0300, Andrius V wrote:

> > > +     int val;
> > > +
> > > +     val = (int) pci_conf_read(sc->as_pc, sc->as_tag, AGP_NVIDIA_0_APSIZE) & 0xff;
> > > +     switch (val) {
> >
> > Why the intermediate val, and the cast?  Why not just:
> >
> >         switch (pci_conf_read(sc->as_pc, sc->as_tag, AGP_NVIDIA_0_APSIZE) &
> >             0xff) {
> >
> 
> I think I felt that it was a very "bulky" statement for a switch key.

I'd agree.  A better variable name than "val" would also help.  For a
drive-by reader it's also not so clear what does 0xff do.  Is it an
expected part of obtaining the `apsize` value, or does does it select
a subfield, or something...  In the one-liner - "APSIZE" is hidden in
the middle of a long line, closer to the rigtht marging, and further
obscured by the all-caps prefix before it and the bitmask after it.
`switch (apsize)` OTOH, is easier to read.

-uwe