On Wed, May 30, 2012 at 12:51:31PM +0100, Ian Abbott wrote: > On 2012-05-29 15:45, Dan Carpenter wrote: > >On Tue, May 29, 2012 at 02:49:35PM +0100, Ian Abbott wrote: > >>Implement the attach_pci() hook as function pc236_attach_pci(). This is > >>called bu comedi_pci_auto_config() in preference to the old attach() > >>hook (implemented by pc236_attach()) and avoids searching for the probed > >>PCI device. > >> > >>Factor out code common to pc236_find_pci() and pc236_attach_pci() into > >>new function pc236_find_pci_board(). Factor out most code common to > >>pc236_attach() and pc236_attach_pci() into new functions > >>pc236_pci_common_attach() and pc236_common_attach(). > >> > >>Also #if out member 'devid' from struct pc236_board unless PCI boards > >>are supported as it is not used for ISA boards. > >> > > > >I can't tell you how much I hate these ifdefs. :( They're every > >where... Who is it that cares about cares so much about 3 extra > >unsigned shorts? > > > >The rules are that #ifdefs are only ok in .h files and not in .c > >files. We should be removing them and not adding them if we want > >to move this code out of staging. > > As an experiment, with the following patch (sorry for the > line-wrapping which prevents it being applied) and with > CONFIG_COMEDI_AMPLC_PC236_ISA defined and > CONFIG_COMEDI_AMPLC_PC236_PCI not defined, the size of the > amplc_pc236.ko increased in size from 5785 bytes to 6215 bytes and > there was a compiler warning about pc236_attach_pci being defined > but not used (this is what the .attach_pci hook in struct > comedi_driver amplc_pc236_driver would get set to, but that is still > conditionally compiled out). If I set the .attach_pci hook to > pc236_attach_pci regardless, it gets rid of the warning, but the > size of the amplc_pc236.ko file increased to 6776 bytes as more code > is reachable in theory although not in practice as the PCI probe > code that calls this function indirectly is still conditionally > compiled out. > I was talking about the devid. And yeah. It shaves of 40 bytes to put the #ifdef there. I can only see three structs that are affected so I thought it would be less. But you you never answered my question about who cares??? The reason this code is in staging is not because it uses a lot of memory but because it's a mess. Before we move it to the stock kernel we will need to remove all the ifdefs out of the .c files. That whole amplc_pc236.c file is riddled with nasty nasty nasty. And no it's not the right idea to sed replace them if (IS_ENABLED()). regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
