Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030322AbbEONdX (ORCPT ); Fri, 15 May 2015 09:33:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57175 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030276AbbEONdV (ORCPT ); Fri, 15 May 2015 09:33:21 -0400 Message-ID: <5555F599.60908@redhat.com> Date: Fri, 15 May 2015 15:33:13 +0200 From: Denys Vlasenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Giuliano Pochini CC: Takashi Iwai , Jaroslav Kysela , LKML Subject: Re: Object code duplication in sound/pci/echoaudio/ References: <55510DE0.3080909@redhat.com> <20150514223206.04d6b79e@wc1> In-Reply-To: <20150514223206.04d6b79e@wc1> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2487 Lines: 65 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). -- 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/