Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752706AbbERIzk (ORCPT ); Mon, 18 May 2015 04:55:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49656 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752561AbbERIzb (ORCPT ); Mon, 18 May 2015 04:55:31 -0400 Date: Mon, 18 May 2015 10:55:29 +0200 Message-ID: From: Takashi Iwai To: Denys Vlasenko Cc: Giuliano Pochini , LKML Subject: Re: Object code duplication in sound/pci/echoaudio/ In-Reply-To: <5555F599.60908@redhat.com> References: <55510DE0.3080909@redhat.com> <20150514223206.04d6b79e@wc1> <5555F599.60908@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.4 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2999 Lines: 79 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 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). thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/