Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200118050410.257697-1-alainm@chromium.org> <37FDD376-5D3D-484C-9209-B59FACA22D0A@holtmann.org>
 <CALWDO_XWvfzRPQ0Ybnpe-x+mw-amrTOMP=PjUpDg0jnXVFRi-w@mail.gmail.com>
 <F6B3CCC4-592C-4F9B-A87B-AC1FF1CB9A78@holtmann.org> <CALWDO_VdPWaSa8MF-y5wB+h9EV9+=vSaPWXu64F+p0crqO84OA@mail.gmail.com>
 <6EF3C3FB-89A6-4B23-A4F5-ADC59FFB9F98@holtmann.org>
In-Reply-To: <6EF3C3FB-89A6-4B23-A4F5-ADC59FFB9F98@holtmann.org>
From:   Alain Michaud <alainmichaud@google.com>
Date:   Thu, 23 Jan 2020 14:04:14 -0500
Message-ID: <CALWDO_UpVApmeKy=679yGM7-Z62Kbd9Pj=9Xyy8FzjC3aHMkkg@mail.gmail.com>
Subject: Re: [PATCH v2] bluetooth: Refactoring mgmt cmd op_code structure
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Alain Michaud <alainm@chromium.org>,
        BlueZ <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

On Thu, Jan 23, 2020 at 1:42 PM Marcel Holtmann <marcel@holtmann.org> wrote=
:
>
> Hi Alain,
>
> > I'll separate the simple one liner into a separate patch then will
> > wait for your suggestion for the rest.  Note that with a single jump
> > table as suggested, it will indeed allow us to defunc some op-codes
> > pretty easily.
>
> I have not fully thought this through yet, but here it goes.
>
> The read_version and read_commands are special since they have to be pres=
ent no matter what. So on purpose we put these two commands first when we d=
esigned the API back in the days. We can special handle them and thus also =
avoid the forward declaration. I am leaning towards this direction since we=
 might want to actually split commands over multiple files to keep this fil=
e readable over time.
>
> Now the downside with putting the opcode as a field in the data structure=
 is that we actually have to look through the every field until we find the=
 opcode and can not just jump to the position defined by that opcode. That =
means actually filling the table with NULL pointers for not supported or di=
sable commands might be better. We could still make the read_commands funct=
ion map the list of supported commands from that table.
That may be an acceptable compromise.  At least there would be exactly
one place to make sure things are aligned.  Perhaps we could also
leave the op-codes in place and create a test that ensures that if
!NULL then op_code =3D=3D index.

>
> This will still not fix the list of supported events since that is still =
static and will require manual patching.
Agreed and actually events are less problematic as there is no jump
table to fix up.  As long as it is in the supported events list, then
it works out pretty well, regardless of order...

>
> Regards
>
> Marcel
>