Messages in this thread | | | Date | Tue, 25 Feb 2020 19:15:41 +0100 | From | Willy Tarreau <> | Subject | Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS |
| |
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@linux.com> 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
| |