[FFmpeg-devel] [PATCH] strict-aliasing-safe aes.c

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:

> On Tue, Jun 29, 2010 at 04:58:33PM +0100, M?ns Rullg?rd wrote:
>> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
>> 
>> > Index: libavutil/aes.h
>> > ===================================================================
>> > --- libavutil/aes.h	(revision 23881)
>> > +++ libavutil/aes.h	(working copy)
>> > @@ -27,12 +27,19 @@
>> >  
>> >  struct AVAES;
>> >  
>> > +typedef union {
>> > +    uint64_t u64[2];
>> > +    uint32_t u32[4];
>> > +    uint8_t u8x4[4][4];
>> > +    uint8_t u8[16];
>> > +} av_aes_block;
>> > +
>> >  /**
>> >   * Initializes an AVAES context.
>> >   * @param key_bits 128, 192 or 256
>> >   * @param decrypt 0 for encryption, 1 for decryption
>> >   */
>> > -int av_aes_init(struct AVAES *a, const uint8_t *key, int key_bits, int decrypt);
>> > +int av_aes_init(struct AVAES *a, const av_aes_block *key, int key_bits, int decrypt);
>> >  
>> >  /**
>> >   * Encrypts / decrypts.
>> > @@ -42,6 +49,7 @@
>> >   * @param iv initialization vector for CBC mode, if NULL then ECB will be used
>> >   * @param decrypt 0 for encryption, 1 for decryption
>> >   */
>> > -void av_aes_crypt(struct AVAES *a, uint8_t *dst, const uint8_t *src, int count, uint8_t *iv, int decrypt);
>> > +void av_aes_crypt(struct AVAES *a, av_aes_block *dst, const av_aes_block *src,
>> > +                  int count, av_aes_block *iv, int decrypt);
>> >  
>> >  #endif /* AVUTIL_AES_H */
>> 
>> Are these API changes necessary?  Can't the rest of the fixes be done
>> without this?
>
> No, they are not necessary, however I am in favour of at least having
> av_aes_block available to allow for easy allocation of data
> with suitable alignment.

Exposing internals like that is less than ideal.  Simply documenting
the 8-byte alignment requirement should be enough.

> (well, and not doing them would mean adding an additional cast to
> av_aes_block * into a local variable I guess).

That is acceptable.

-- 
M?ns Rullg?rd
mans at mansr.com