Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h

[Richard Henderson]

On 8/17/20 1:42 PM, Richard Henderson wrote:
> On 8/17/20 1:38 PM, Richard Henderson wrote:
>> On 8/17/20 9:48 AM, Daniel P. Berrangé wrote:
>>> On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
>>>> This allows header files to declare pointers without pulling
>>>> in the entire crypto subsystem.
>>>>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>  include/crypto/cipher.h | 2 --
>>>>  include/qemu/typedefs.h | 1 +
>>>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> I'm not in favour of this change or the next. Using #include "cipher.h"
>>> is not a burden on the users of the crypto code. Moving typedefs away
>>> from the associated struct is a step backwards IMHO.
>>
>> Consider if you put a pointer to QCryptoCipher in a relatively generic header
>> file (e.g. "target/foo/cpu.h"), restricting "cipher.h" to a portion of the
>> implementation (e.g. target/foo/helper_crypto.c).
>>
>> This sort of thing is exactly why "qemu/typedefs.h" exists.
> 
> As for the next patch for QCryptoCipherDriver, I could easily see not moving
> the typedef to typedefs.h, but instaed to "crypto.h", where we do in fact want
> to declare an incomplete structure.  I think it's a real mistake to be using
> void* there at present.

That said, I can drop this first patch because, in the end, I'm *not* going to
put QCryptoCipher in target/arm/cpu.h.


r~