Re: [PATCH 5/12: eCryptfs] Header declarations

[James Morris <jmorris@namei.org>]

On Wed, 2 Nov 2005, Phillip Hellewell wrote:

> +struct ecryptfs_password {
> +	char password[ECRYPTFS_MAX_PASSWORD_LENGTH];
> +	int password_size;
> +	unsigned char salt[ECRYPTFS_SALT_SIZE];
> +	int saltless;		/* If set, this is the ``seed'' token from which
> +				 * other salted tokens are derived. Note that
> +				 * this is _not_ the same as a token that just
> +				 * has not received its salt yet. */
> +	int hash_algo;
> +	int hash_iterations;
> +	/* Iterated-hash concatenation of salt and passphrase */
> +	unsigned char session_key_encryption_key[ECRYPTFS_MAX_KEY_BYTES];
> +	int session_key_encryption_key_size;	/* In bytes */
> +	int session_key_encryption_key_set;
> +	/* Always in expanded hex */
> +	unsigned char signature[ECRYPTFS_PASSWORD_SIG_SIZE + 1];
> +};

Generally, you should put the smallest components of a data structure at 
the front, so the compiler can optimize alignment.

> +#define RECORDS_PER_PAGE(crypt_stats) (crypt_stats->extent_size / \
> +                                       (crypt_stats->iv_bytes))
> +#define PG_IDX_TO_LWR_PG_IDX(crypt_stats, idx) \
> +        ((idx / crypt_stats->records_per_page) + idx \
> +         + crypt_stats->num_header_pages + 1)
> +#define RECORD_IDX(crypt_stats, idx) (idx % crypt_stats->records_per_page)
> +#define RECORD_OFFSET(crypt_stats, idx) \
> +        (RECORD_IDX(crypt_stats, idx) * (crypt_stats->iv_bytes \
> +                                         + crypt_stats->hmac_bytes))
> +#define LAST_RECORDS_PAGE_IDX(crypt_stats, idx) \
> +        (PG_IDX_TO_LWR_PG_IDX(crypt_stats, idx) \
> +         - RECORD_IDX(crypt_stats,idx) - 1)

These should all be static inline (and any other similar).

> + */
> +#define FILE_TO_PRIVATE(file) ((struct ecryptfs_file_info *)((file)->private_data))
> +#define FILE_TO_PRIVATE_SM(file) ((file)->private_data)
> +#define FILE_TO_LOWER(file) ((FILE_TO_PRIVATE(file))->wfi_file)
> +#define INODE_TO_PRIVATE(ino) ((struct ecryptfs_inode_info *)(ino)->u.generic_ip)
> +#define INODE_TO_PRIVATE_SM(ino) ((ino)->u.generic_ip)
> +#define INODE_TO_LOWER(ino) (INODE_TO_PRIVATE(ino)->wii_inode)
> +#define SUPERBLOCK_TO_PRIVATE(super) ((struct ecryptfs_sb_info *)(super)->s_fs_info)
> +#define SUPERBLOCK_TO_PRIVATE_SM(super) ((super)->s_fs_info)
> +#define SUPERBLOCK_TO_LOWER(super) (SUPERBLOCK_TO_PRIVATE(super)->wsi_sb)
> +#define DENTRY_TO_PRIVATE_SM(dentry) ((dentry)->d_fsdata)
> +#define DENTRY_TO_PRIVATE(dentry) ((struct ecryptfs_dentry_info *)(dentry)->d_fsdata)
> +#define DENTRY_TO_LOWER(dentry) (DENTRY_TO_PRIVATE(dentry)->wdi_dentry)

These macro names are too generic.


- James
-- 
James Morris
<jmorris@namei.org>