| Message ID | 1410766234-1634-2-git-send-email-behanw@converseincode.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 15, 2014 at 12:30:23AM -0700, behanw@converseincode.com wrote: > From: Behan Webster <behanw@converseincode.com> > > Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS) > with a C99 compliant equivalent. This macro instead allocates the appropriate > amount of memory using an char array. > > The new code can be compiled with both gcc and clang. > > struct shash_desc contains a flexible array member member ctx declared with > CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning > of the array declared after struct shash_desc with long long. > > No trailing padding is required because it is not a struct type that can > be used in an array. > > The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long > as would be the case for a struct containing a member with > CRYPTO_MINALIGN_ATTR. > > Signed-off-by: Behan Webster <behanw@converseincode.com> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Thanks,
On 17/09/14 12:22, Herbert Xu wrote: > On Mon, Sep 15, 2014 at 12:30:23AM -0700, behanw@converseincode.com wrote: >> From: Behan Webster <behanw@converseincode.com> >> >> Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS) >> with a C99 compliant equivalent. This macro instead allocates the appropriate >> amount of memory using an char array. >> >> The new code can be compiled with both gcc and clang. >> >> struct shash_desc contains a flexible array member member ctx declared with >> CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning >> of the array declared after struct shash_desc with long long. >> >> No trailing padding is required because it is not a struct type that can >> be used in an array. >> >> The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long >> as would be the case for a struct containing a member with >> CRYPTO_MINALIGN_ATTR. >> >> Signed-off-by: Behan Webster <behanw@converseincode.com> > Acked-by: Herbert Xu <herbert@gondor.apana.org.au> > > Thanks, Just in case. I would still follow advice from "Michał Mirosław" to use shash##__desc[] - Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Sep 17, 2014 at 02:15:40PM +0300, Dmitry Kasatkin wrote: > On 17/09/14 12:22, Herbert Xu wrote: > > On Mon, Sep 15, 2014 at 12:30:23AM -0700, behanw@converseincode.com wrote: > >> From: Behan Webster <behanw@converseincode.com> > >> > >> Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS) > >> with a C99 compliant equivalent. This macro instead allocates the appropriate > >> amount of memory using an char array. > >> > >> The new code can be compiled with both gcc and clang. > >> > >> struct shash_desc contains a flexible array member member ctx declared with > >> CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning > >> of the array declared after struct shash_desc with long long. > >> > >> No trailing padding is required because it is not a struct type that can > >> be used in an array. > >> > >> The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long > >> as would be the case for a struct containing a member with > >> CRYPTO_MINALIGN_ATTR. > >> > >> Signed-off-by: Behan Webster <behanw@converseincode.com> > > Acked-by: Herbert Xu <herbert@gondor.apana.org.au> > > > > Thanks, > > Just in case. > I would still follow advice from "Michał Mirosław" to use shash##__desc[] Oh yes of course. My ack is more about the approach. Cheers,
On 09/17/14 04:30, Herbert Xu wrote: > On Wed, Sep 17, 2014 at 02:15:40PM +0300, Dmitry Kasatkin wrote: >> On 17/09/14 12:22, Herbert Xu wrote: >>> On Mon, Sep 15, 2014 at 12:30:23AM -0700, behanw@converseincode.com wrote: >>>> From: Behan Webster <behanw@converseincode.com> >>>> >>>> Add a macro which replaces the use of a Variable Length Array In Struct (VLAIS) >>>> with a C99 compliant equivalent. This macro instead allocates the appropriate >>>> amount of memory using an char array. >>>> >>>> The new code can be compiled with both gcc and clang. >>>> >>>> struct shash_desc contains a flexible array member member ctx declared with >>>> CRYPTO_MINALIGN_ATTR, so sizeof(struct shash_desc) aligns the beginning >>>> of the array declared after struct shash_desc with long long. >>>> >>>> No trailing padding is required because it is not a struct type that can >>>> be used in an array. >>>> >>>> The CRYPTO_MINALIGN_ATTR is required so that desc is aligned with long long >>>> as would be the case for a struct containing a member with >>>> CRYPTO_MINALIGN_ATTR. >>>> >>>> Signed-off-by: Behan Webster <behanw@converseincode.com> >>> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> >>> >>> Thanks, >> Just in case. >> I would still follow advice from "Michał Mirosław" to use shash##__desc[] Absolutely. I will be posting a v4 patchset . Just waiting a bit more for more comments on v3. The macro from v4 will look like this which I believe will satisfy the concern and indeed be safer than my previous version. +#define SHASH_DESC_ON_STACK(shash, tfm) \ + char __##shash##_desc[sizeof(struct shash_desc) + \ + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \ + struct shash_desc *shash = (struct shash_desc *)__##shash##_desc Hmm. Is it worth adding a comment with this macro explaining the reason this works? Essentially much of what is in the commit message? > Oh yes of course. My ack is more about the approach. Wonderful! Indeed. I would have asked for you to wait for v4 anyways. :) Thank you, Behan
diff --git a/include/crypto/hash.h b/include/crypto/hash.h index a391955..541125b 100644 --- a/include/crypto/hash.h +++ b/include/crypto/hash.h @@ -58,6 +58,11 @@ struct shash_desc { void *__ctx[] CRYPTO_MINALIGN_ATTR; }; +#define SHASH_DESC_ON_STACK(shash, tfm) \ + char __desc[sizeof(struct shash_desc) + \ + crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; \ + struct shash_desc *shash = (struct shash_desc *)__desc + struct shash_alg { int (*init)(struct shash_desc *desc); int (*update)(struct shash_desc *desc, const u8 *data,