Hi Stephan,

thanks for your fast response.

> Please run checkpatch.pl on the patch and fix the formatting issues.

I've run checkpatch.pl again and fixed all errors and warnings except the 
warnings about printk.
Compression does not have it's own subsystem, that is why I used 
printk(KERN_INFO ...

> In general: I do not think that having larger C functions in header files is 
> a 
> proper coding style. 

How should I solve this?

Option 1:
Move everything in the lib/zbewalgo folder into a single source file.
This way there is no function defined in a header file.
I separated the code into different files because the different partial 
compression algorithms are independent from each other.

Option 2:
Move each header file in its own *.c file, while keeping the 
function-declarations in the header.
If the code is scattered in multiple source files each of the partial 
algorithms would show up as an independent module.
All these modules would load simultaneously with the module zbewalgo_compress
The module zbewalgo_compress requires an array of all partial algorithms.
This would spam the 'lsmod' list with unneccessary details.

I would prefer option 1, since it adds less overhead.

> Also, having static variables header files is also not 
> nice.

I will remove the static modifier for variables in the header files

> Do not redefine code that already exists. For example, MIN/MAX exists: min_t 
> and max_t.

Ok, I will use min_t and max_t

> Why are there __attribute__ ((unused)) function parameters, such as in 
> compress_bitshuffle and others?

The zbewalgo algorithm is a container-algorithm for compression functions with 
the ability to concatenate multiple algorithms.
To be able to call any compression algorithm the same way, I defined 'struct 
zbewalgo_alg' as the interface for all those independent compression algorithms.
Some of the partial algorithms do not require all parameters. To silence 
compiler warnings (if additional warning flags are enabled) I explicitly add 
the 'unused'-parameter

Best regards,
Benjamin

Reply via email to