Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 25, 2020 at 7:22 AM Denis Efremov <efremov@xxxxxxxxx> wrote:
>
> I think that for the first attempt changing will be enough:
> -static int fdc;                        /* current fdc */
> +static int current_fdc;                        /* current fdc */
> and
> -#define FD_IOPORT fdc_state[fdc].address
> +#define FD_IOPORT fdc_state[current_fdc].address

Please don't do this blindly - ie without verifying that there are no
cases of that "local fdc variable shadowing" issue.

Of course, such a verification might be as easy as "generates exact
same code" rather than looking at every use.

And btw, don't worry too much about this being in an UAPI file. I'm
pretty sure that's because of specialty programs that use the magical
ioctls to do special formatting. They want the special commands
(FD_FORMAT etc), but I don't think they really use the port addresses.

So I think it's in a UAPI file entirely by mistake.

We should at least try moving those bits to the floppy.c file and
remove it from the header file.

For example, doing a Debian code search on "FDPATCHES" doesn't find
any user space hits. Searching for "FD_STATUS" gets a lot of hits, but
thos all seem to be because it's a symbol used by user space programs,
("file descriptor status"), not because those hits actually used the
fdreg.h header file.

So we can remove at least the FD_IOPORT mess from the header file, I bet.

Worst case - if somebody finds some case that uses them, we can put it back.

                Linus



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux