Re: [PATCH] 2.5.44: lkcd (8/9): dump compression files

[Christoph Hellwig <hch@sgi.com>]

On Mon, Oct 21, 2002 at 03:16:03AM -0700, Matt D. Robinson wrote:
> +
> +/*
> + * -----------------------------------------------------------------------
> + *                       D E F I N I T I O N S
> + * -----------------------------------------------------------------------
> + */
> +#define DUMP_MODULE_NAME "dump_gzip"
> +#define DUMP_PRINTN(format, args...) \
> +	printk("%s: " format , DUMP_MODULE_NAME , ## args);

Those two don't seem to be used at all.

> +#define DUMP_PRINT(format, args...) \
> +	printk(format , ## args);

Please just use printk directly.

> +/* setup the gzip compression functionality */
> +static dump_compress_t dump_gzip_compression = {
> +	compress_type:	DUMP_COMPRESS_GZIP,
> +	compress_func:	dump_compress_gzip,
> +};

This want C99-style initializes.

> +
> +/*
> + * Name: dump_compress_gzip_init()
> + * Func: Initialize gzip as a compression mechanism.
> + */
> +int __init
> +dump_compress_gzip_init(void)
ould be static.


> +{
> +	deflate_workspace = vmalloc(zlib_deflate_workspacesize());
> +	if (!deflate_workspace) {
> +		DUMP_PRINT("Failed to alloc %d bytes for deflate workspace\n",
> +			zlib_deflate_workspacesize());
> +		return -ENOMEM;
> +	}
> +	dump_register_compression(&dump_gzip_compression);
> +	return (0);
> +}
> +
> +/*
> + * Name: dump_compress_gzip_cleanup()
> + * Func: Remove gzip as a compression mechanism.
> + */
> +void __exit
> +dump_compress_gzip_cleanup(void)

Dito

> +{
> +	vfree(deflate_workspace);
> +	dump_unregister_compression(DUMP_COMPRESS_GZIP);
> +}
> +
> +/* module initialization */
> +module_init(dump_compress_gzip_init);
> +module_exit(dump_compress_gzip_cleanup);
> +
> +#ifdef MODULE
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,16))
> +MODULE_LICENSE("GPL");
> +#endif
> +#endif /* MODULE */

Please remove the horrible ifdefs.  MODULE_AUTHOR and MODULE_DESCRIPTION
would be nice, btw..

> +/*
> + * Name: dump_compress_rle()
> + * Func: Compress a DUMP_PAGE_SIZE (hardware) page down to something more reasonable,
> + *       if possible.  This is the same routine we use in IRIX.
> + */

Reformat to fit an ANSI terminal?

> +static int
> +dump_compress_rle(char *old, int oldsize, char *new, int newsize)
> +{
> +	int ri, wi, count = 0;
> +	u_char value = 0, cur_byte;

u16

> +/* setup the rle compression functionality */
> +static dump_compress_t dump_rle_compression = {
> +	compress_type:	DUMP_COMPRESS_RLE,
> +	compress_func:	dump_compress_rle,
> +};

Again, C99-initializers.

> +
> +/*
> + * Name: dump_compress_rle_init()
> + * Func: Initialize rle compression for dumping.
> + */
> +int __init
> +dump_compress_rle_init(void)

Again static

> +{
> +	dump_register_compression(&dump_rle_compression);
> +	return (0);

Shouldn't dump_register_compression return æn error that
you should return?  Also return 0 instead of return (0)

> +/*
> + * Name: dump_compress_rle_cleanup()
> + * Func: Remove rle compression for dumping.
> + */
> +void __exit
> +dump_compress_rle_cleanup(void)
> +{

Static again.

> +	dump_unregister_compression(DUMP_COMPRESS_RLE);
> +}
> +
> +/* module initialization */
> +module_init(dump_compress_rle_init);
> +module_exit(dump_compress_rle_cleanup);
> +
> +#ifdef MODULE
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,16))
> +MODULE_LICENSE("GPL");
> +#endif
> +#endif /* MODULE */

As above.

Btw, any chance the actual algorithm could go into lib/?