Re: [PATCH] crypto: eip93 - use struct_size() and flexible array for ring allocation

[Rosen Penev]

On Tue, Aug 4, 2026 at 12:47 AM Thomas Huth <thuth@xxxxxxxxxx> wrote:
>
> On 04/08/2026 09.18, Rosen Penev wrote:
> > On Tue, Aug 4, 2026 at 12:11 AM Thomas Huth <thuth@xxxxxxxxxx> wrote:
> >>
> >> On 04/08/2026 00.40, Rosen Penev wrote:
> >>> Embed the single ring as a flexible array member in eip93_device
> >>> instead of allocating it separately. This simplifies the probe path
> >>> and uses struct_size() for a single allocation.
> >>>
> >>> Assisted-by: opencode:big-pickle
> >>> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> >>> ---
> >>>    .../crypto/inside-secure/eip93/eip93-main.c   |  6 +----
> >>>    .../crypto/inside-secure/eip93/eip93-main.h   | 22 +++++++++----------
> >>>    2 files changed, 12 insertions(+), 16 deletions(-)
> >>>
> >>> diff --git a/drivers/crypto/inside-secure/eip93/eip93-main.c b/drivers/crypto/inside-secure/eip93/eip93-main.c
> >>> index 1a8dabc4ada4..e62785952b0d 100644
> >>> --- a/drivers/crypto/inside-secure/eip93/eip93-main.c
> >>> +++ b/drivers/crypto/inside-secure/eip93/eip93-main.c
> >>> @@ -415,7 +415,7 @@ static int eip93_crypto_probe(struct platform_device *pdev)
> >>>        u32 ver, algo_flags;
> >>>        int ret;
> >>>
> >>> -     eip93 = devm_kzalloc(dev, sizeof(*eip93), GFP_KERNEL);
> >>> +     eip93 = devm_kzalloc(dev, struct_size(eip93, ring, 1), GFP_KERNEL);
> >>>        if (!eip93)
> >>>                return -ENOMEM;
> >>>
> >>> @@ -436,10 +436,6 @@ static int eip93_crypto_probe(struct platform_device *pdev)
> >>>        if (ret)
> >>>                return ret;
> >>>
> >>> -     eip93->ring = devm_kcalloc(eip93->dev, 1, sizeof(*eip93->ring), GFP_KERNEL);
> >>> -     if (!eip93->ring)
> >>> -             return -ENOMEM;
> >>> -
> >>>        ret = eip93_desc_init(eip93);
> >>>        if (ret)
> >>>                return ret;
> >>> diff --git a/drivers/crypto/inside-secure/eip93/eip93-main.h b/drivers/crypto/inside-secure/eip93/eip93-main.h
> >>> index 990c2401b7ce..5f0f51081743 100644
> >>> --- a/drivers/crypto/inside-secure/eip93/eip93-main.h
> >>> +++ b/drivers/crypto/inside-secure/eip93/eip93-main.h
> >>> @@ -92,17 +92,6 @@
> >>>                                                    EIP93_HASH_SHA224 | \
> >>>                                                    EIP93_HASH_SHA256))
> >>>
> >>> -/**
> >>> - * struct eip93_device - crypto engine device structure
> >>> - */
> >>> -struct eip93_device {
> >>> -     void __iomem            *base;
> >>> -     struct device           *dev;
> >>> -     struct clk              *clk;
> >>> -     int                     irq;
> >>> -     struct eip93_ring               *ring;
> >>> -};
> >>> -
> >>>    struct eip93_desc_ring {
> >>>        void                    *base;
> >>>        void                    *base_end;
> >>> @@ -131,6 +120,17 @@ struct eip93_ring {
> >>>        struct idr                      crypto_async_idr;
> >>>    };
> >>>
> >>> +/**
> >>> + * struct eip93_device - crypto engine device structure
> >>> + */
> >>> +struct eip93_device {
> >>> +     void __iomem            *base;
> >>> +     struct device           *dev;
> >>> +     struct clk              *clk;
> >>> +     int                     irq;
> >>> +     struct eip93_ring       ring[];
> >>> +};
> >> This looks weird, too. If there is always only one "ring", why don't you
> >> embed it without the "[]" into the struct eip93_device directly?
> > keeps all callers the same. -> vs .
> So it's basically keeping the patch small and generating many WTFs for
> future reviewers of the code vs. having a bigger patch now and better
> understable code in the future. Not my decision (it's up to the
> maintainers), but FWIW I'd rather go with option 2.
>
> Anyway, if you want to keep it short, wouldn't it also be possible to
> declare it as ring[1] instead and then keep the sizeof() instead of the
> struct_size() ?
No because [1] arrays are deprecated.
>
>   Thomas
>