Return-Path: <cyrus@holtmann.org>
Subject: Re: Bluez coding style again
From: Marcel Holtmann <marcel@holtmann.org>
To: Siarhei Siamashka <siarhei.siamashka@nokia.com>
Cc: BlueZ development <linux-bluetooth@vger.kernel.org>
In-Reply-To: <200901171943.45370.siarhei.siamashka@nokia.com>
References: <200901171943.45370.siarhei.siamashka@nokia.com>
Content-Type: text/plain
Date: Sun, 18 Jan 2009 16:08:12 +0100
Message-Id: <1232291292.5095.9.camel@californication>
Mime-Version: 1.0
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Siarhei,

> > commit 1fb5ffff0b704038a90e03860ecce63be2deb988
> > Author: Johan Hedberg <johan.hedberg@nokia.com>
> > Date:   Fri Jan 16 20:29:43 2009 +0200
> >
> >     Fix indentation to use only tabs
> >
> > --- a/sbc/sbc_tables.h
> > +++ b/sbc/sbc_tables.h
> > @@ -161,30 +161,30 @@ static const int32_t synmatrix8[16][8] = {
> >         ((FIXED_A) 1 << (sizeof(FIXED_T) * CHAR_BIT - 1)) + 0.5)
> >  #define F(x) F_PROTO4(x)
> >  static const FIXED_T _sbc_proto_fixed4[40] = {
> > -        F(0.00000000E+00),  F(5.36548976E-04),
> > +       F(0.00000000E+00),  F(5.36548976E-04),
> >         -F(1.49188357E-03),  F(2.73370904E-03),
> > -        F(3.83720193E-03),  F(3.89205149E-03),
> > -        F(1.86581691E-03),  F(3.06012286E-03),
> > +       F(3.83720193E-03),  F(3.89205149E-03),
> > +       F(1.86581691E-03),  F(3.06012286E-03),
> 
> With this latest commit, the vertical alignment of elements in tables gets
> messed up and it becomes harder to see any kind of symmetry in the tables.
> If the spaces are strictly forbidden, I would probably prefer replacing them
> with '+' character.

that would be fine with me.

> Anyway, with this requirement also in effect, it makes harder to use the
> standard 'checkpatch.pl' from linux distribution as you have to watch for
> a lot more stuff which can get through.
> 
> Adding --strict option to 'checkpatch.pl' only also reports something like:
> 
> CHECK: multiple assignments should be avoided
> #623: FILE: sbc/sbc_primitives.c:381:
> +       x[159] = x[31] = pcm[0 + 2];
> 
> CHECK: architecture specific defines should be avoided
> #475: FILE: sbc/sbc_primitives_mmx.h:31:
> +#if defined(__GNUC__) && (defined(__i386__) || defined(__amd64__)) && \
> 
> 
> I have learned the following extra bluez coding style requirements over the
> standard linux coding style up to now:
> 1. Space is required for cast operation, ex. '(int) x'
> 2. Indentation between '#' and 'define' is forbidden
> 3. Any kind of leading spaces are forbidden
> 
> Have I missed something else?
> 
> Anyway, does it make sense to introduce a modified variant of 'checkpatch.pl'?
> Or probably somebody has done it already, but did not care to share with
> everyone else? :)

Not sure about checkpatch.pl, but the BlueZ coding style is mainly
enforced by Johan and me. In some cases you have to break, but that
should only happen if there is no other choice. 99% of the time you just
can simplify your code. However I happily admit that with codec work
this might not always be true.

Regards

Marcel