On Tue, 2011-09-20 at 08:05 -0500, Bill Kendall wrote:
> Various structures in a dump file optionally contain a checksum, but
> the code to compute and validate the checksum has not been enabled.
> The checksum code has a negligible performance impact and so this
> patch enables the checksum code unconditionally. Also:
>
> - make sure all header sizes are multiples of 4 bytes
> (a requirement of the checksum routine)
> - zero structures to ensure internal padding has a known value
> - fix a bug in dump_extattr_buildrecord() which checksummed
> the wrong header structure
> - add calc_checksum() and is_checksum_valid() routines to
> cut down on duplicate code
>
> Signed-off-by: Bill Kendall <wkendall@xxxxxxx>
This looks good. I have a few comments for you
below but unless you decide to send me an update
I'll use this as-is.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
> ---
> common/content_inode.h | 25 ++++++++++++++
> dump/content.c | 85 +++++++++++-------------------------------------
> restore/Makefile | 2 +-
> restore/content.c | 40 ++--------------------
> 4 files changed, 49 insertions(+), 103 deletions(-)
>
> diff --git a/common/content_inode.h b/common/content_inode.h
> index 479fdfc..9c2c1cc 100644
> --- a/common/content_inode.h
> +++ b/common/content_inode.h
> @@ -347,4 +347,29 @@ typedef struct extattrhdr extattrhdr_t;
> /* a linux "secure" mode attribute
> */
>
> +/* Routines for calculating and validating checksums on xfsdump headers.
> + * The header length must be an integral number of u_int32_t's.
> + */
> +static inline u_int32_t
> +calc_checksum(void *bufp, size_t len)
> +{
> + u_int32_t sum = 0;
> + u_int32_t *sump = (u_int32_t *)bufp;
> + u_int32_t *endp = (void *)sump + len
No need to cast a (void *) object to another
pointer type (and vice-versa).
And although gcc allows arithmetic on void pointers,
it is not standard, so (char *) would be a more
portable choice.
The multiple-of-4 assumption would be well stated with
an assertion.
> ;
> + while (sump < endp)
> + sum += *sump++;
> + return ~sum + 1;
> +}
> +
> +static inline bool_t
> +is_checksum_valid(void *bufp, size_t len)
> +{
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
