Messages in this thread | | | Date | Fri, 15 May 2015 15:33:13 +0200 | From | Denys Vlasenko <> | Subject | Re: Object code duplication in sound/pci/echoaudio/ |
| |
On 05/14/2015 10:32 PM, Giuliano Pochini wrote: > On Mon, 11 May 2015 22:15:28 +0200 > Denys Vlasenko <dvlasenk@redhat.com> wrote: > >> There are fourteen files in sound/pci/echoaudio/, namely: >> >> darla20.c darla24.c echo3g.c gina20.c gina24.c indigo.c >> indigodj.c indigodjx.c indigoio.c indigoiox.c layla20.c >> layla24.c mia.c mona.c >> >> Which use the following method of "code reuse": >> >> #include "echoaudio_dsp.c" >> #include "echoaudio_gml.c" >> #include "echoaudio.c" >> >> echoaudio.c is not a header file, it contains a bunch of >> static functions, some of a considerable size. >> This makes those functions to be duplicated many times over. > > Not really. It is quite unlikely that there are two different Echoaudio cards installed.
Distros tend to build almost all drivers in the tree. On Fedora, kernel has a config where the following drivers are built from sources in sound/pci/echoaudio/:
$ ls /lib/modules/4.0.0/kernel/sound/pci/echoaudio/ snd-darla20.ko snd-gina20.ko snd-indigodjx.ko snd-indigo.ko snd-mia.ko snd-darla24.ko snd-gina24.ko snd-indigoio.ko snd-layla20.ko snd-mona.ko snd-echo3g.ko snd-indigodj.ko snd-indigoiox.ko snd-layla24.ko
>> For instance, there are fourteen instances of init_engine(), >> each 1117 bytes long. Fourteen instances of pcm_open(), each 556 bytes >> long. >> >> 11 get_firmware >> 10 free_firmware >> 13 audiopipe_free >> 14 init_hw >> 14 hw_rule_capture_format_by_channels >> 14 hw_rule_capture_channels_by_format >> 14 hw_rule_playback_format_by_channels >> 14 hw_rule_playback_channels_by_format >> >> and so on. >> >> In my humble opinion, this is not a good coding practice. >> You should not duplicate functions like this. >> Where possible, you need to reuse a single instance of a function. > > One option is to make a single driver which supports all the cards. > There is not any duplicated code, but there is a lot of unused code. > The other way (the one I choosed) is to build many specialized drivers.
If you have a lot of common code, you just need to have it factored out into a separate module, say, echoaudio-core.c, and make individual drivers depend on it.
This is done all over the kernel (there are nearly 400 *core*.c files in the tree).
| |