Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756075AbYKNT0V (ORCPT ); Fri, 14 Nov 2008 14:26:21 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754371AbYKNT0L (ORCPT ); Fri, 14 Nov 2008 14:26:11 -0500 Received: from ik-out-1112.google.com ([66.249.90.179]:45544 "EHLO ik-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752174AbYKNT0K (ORCPT ); Fri, 14 Nov 2008 14:26:10 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=CXMAMWSGd8NpVDZKS4jM3Vm/Rk3C1KnEznMZC4r6sqEMKha2++Nw4RS9Jfqfo5PVtI zuM6lLtb/cybVrcwaMkgBtM2iMzw5T1CYpWJivaeW6SpbDHraWPZ8ph1KRiRRWUOTGsa aWAxlTsaCp3juDO5SR4H6R1UI/DiPZC8mkMcw= Message-ID: <412bdbff0811141125x34de747dm85b47f61b604219a@mail.gmail.com> Date: Fri, 14 Nov 2008 14:25:57 -0500 From: "Devin Heitmueller" To: "Harvey Harrison" Subject: Re: [v4l-dvb-maintainer] [PATCH] dvb: usb vendor_ids/product_ids are __le16 Cc: "Michael Krufky" , "Mauro Carvalho Chehab" , v4l-maintainer , "Andrew Morton" , LKML In-Reply-To: <1226690438.5483.39.camel@brick> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1226686796.5483.33.camel@brick> <491DCBDD.4000703@linuxtv.org> <1226689669.5483.36.camel@brick> <37219a840811141115g539ef85fo81136ebd479f016f@mail.gmail.com> <1226690438.5483.39.camel@brick> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1847 Lines: 49 On Fri, Nov 14, 2008 at 2:20 PM, Harvey Harrison wrote: > On Fri, 2008-11-14 at 14:15 -0500, Michael Krufky wrote: >> On Fri, Nov 14, 2008 at 2:07 PM, Harvey Harrison >> wrote: >> > On Fri, 2008-11-14 at 14:05 -0500, Michael Krufky wrote: >> >> Wouldn't it be nicer to just switch on >> >> cpu_to_le16(udev->descriptor.idVendor) ? This would be a 1-line change, >> >> compile to a smaller footprint, and be easier to read. >> >> >> >> Personally, I prefer to try to avoid duplicating code in places where a >> >> single operation may occur centrally. >> > >> > On a little-endian arch it makes no difference obviously, but on a >> > big-endian arch it's the difference between compile-time and runtime >> > byteswapping. >> > >> >> Its not my driver, but I think that doing the operation on the switch >> is prettier than doing it on each case statement. Also, it would >> avoid future bugs, if somebody decides to add new cases to the switch >> block. > > The alternative is to define the vendor ids as little-endian in the headers > then you don't need the endian swap in the switch or the case, but that would > require looking at the other uses first....or gently encourage more people to run sparse ;-) > > Harvey Harvey, If I may offer my opinion, this is not remotely performance-critical code. That said, I would favor on the side of maintainability/reliability in this case, and mkrufky's approach seems to be better on both accounts. Regards, Devin -- Devin J. Heitmueller http://www.devinheitmueller.com AIM: devinheitmueller -- 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/