| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 20 May 2015 01:05:05 +0200 From: Giuliano Pochini <pochini@...ny.it> To: Takashi Iwai <tiwai@...e.de> Cc: Denys Vlasenko <dvlasenk@...hat.com>, LKML <linux-kernel@...r.kernel.org> Subject: Re: Object code duplication in sound/pci/echoaudio/ On Mon, 18 May 2015 10:55:29 +0200 Takashi Iwai <tiwai@...e.de> wrote: > At Fri, 15 May 2015 15:33:13 +0200, > Denys Vlasenko wrote: > > > > On 05/14/2015 10:32 PM, Giuliano Pochini wrote: > > > On Mon, 11 May 2015 22:15:28 +0200 > > > Denys Vlasenko <dvlasenk@...hat.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). > > Yes, we know of such problems well. But practically seen, the only > problem is the disk space, so it's been in a low priority of TODO > list. There are a few other drivers having the very same issue, and > the biggest problem is the lack of test hardware, as they tend to be > legacy boards (either ISA or old PCI). Ok, I'll try to find the time to split the drivers. Do not expect it will be ready tomorrow :) -- Giuliano. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists