linux-kernel - Re: [PATCH v5 03/16] media: rkisp1: Add user space ABI definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d1677c06-4e9f-58a9-817d-0a4b8d958285@xs4all.nl>
Date:   Fri, 9 Mar 2018 14:14:00 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Jacob Chen <jacobchen110@...il.com>
Cc:     Shunqian Zheng <zhengsq@...k-chips.com>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Tomasz Figa <tfiga@...omium.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        钟以崇 <zyc@...k-chips.com>,
        Eddie Cai <eddie.cai.linux@...il.com>,
        Jeffy Chen <jeffy.chen@...k-chips.com>,
        Allon Huang <allon.huang@...k-chips.com>,
        devicetree@...r.kernel.org, Heiko Stuebner <heiko@...ech.de>,
        robh+dt@...nel.org, Joao Pinto <Joao.Pinto@...opsys.com>,
        Luis Oliveira <Luis.Oliveira@...opsys.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>
Subject: Re: [PATCH v5 03/16] media: rkisp1: Add user space ABI definitions

On 08/03/18 11:00, Jacob Chen wrote:
> Hi Hans,
> 
> 2018-02-07 20:00 GMT+08:00 Hans Verkuil <hverkuil@...all.nl>:
>> On 12/29/17 08:52, Shunqian Zheng wrote:
>>> From: Jeffy Chen <jeffy.chen@...k-chips.com>
>>>
>>> Add the header for userspace
>>
>> General note: I saw four cases where this documentation referred to the
>> datasheet. Three comments on that:
>>
>> 1) You don't say which datasheet.
>> 2) I assume the datasheet is under NDA?
> 
> This datasheet can't be got by customers, even under NDA.

OK. This should be mentioned somewhere.

> 
>> 3) You do need to give enough information so a reasonable default can be
>>    used. I mentioned in an earlier review that creating an initial params
>>    struct that can be used as a templete would be helpful (or even
>>    required), and that would be a good place to put such defaults.
>>
> 
> It don't need a default config
> For applcation writers, they can just init it with zero data, and only
> set value for the part they concerned.

This too should be clearly documented. This is very useful information for
users of this API.

> 
> As for ABI, i have checked there is no mismatches.
> Those structures is 32 bit aligned both in 64bit/32bit env, since
> there is no 64bit value.
> "__attribute__ ((packed))" can avoid mismatches happen when we add a
> 64bit value to those structures.
> 
> As robin said, enums and bools are not guaranteed to be consistent
> between different compiler, so it's a potential risk.
> I have replace bools with unsigned char and enums with unsigned int.

Excellent.

Regards,

	Hans