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

[Willy Tarreau]

On Tue, Feb 25, 2020 at 10:08:51AM -0800, Linus Torvalds wrote:
> 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.

That's exactly what I'm doing. In fact I first renamed the variable
and am manually checking all places which do not compile anymore.
Hence the surprizes.

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

That's exactly what I'm doing.

> 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.

OK this will help me, thanks for the hint :-)

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

Makes sense.

> 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.

I like that. And at least we'll know how they use it (likely without the
dependency on fdc).

Thanks,
Willy