Re: [PATCH] device-mapper: multipath hardware handler

[Ingo Oeser <ioe-lkml@axxeo.de>]

Hi Alasdair,

Alasdair G Kergon wrote:
> +/*
> + * Constructs a hardware handler object, takes custom arguments
> + */
> +typedef int (*hwh_ctr_fn) (struct hw_handler *hwh, unsigned arc, char
> **argv); +typedef void (*hwh_dtr_fn) (struct hw_handler *hwh);
> +
> +typedef void (*hwh_pg_init_fn) (struct hw_handler *hwh, unsigned bypassed,
> +    struct path *path);
> +typedef unsigned (*hwh_err_fn) (struct hw_handler *hwh, struct bio *bio);
> +typedef int (*hwh_status_fn) (struct hw_handler *hwh,
> +         status_type_t type,
> +         char *result, unsigned int maxlen);
> +
> +/* Information about a hardware handler type */
> +struct hw_handler_type {
> + char *name;
> + struct module *module;
> +
> + hwh_ctr_fn ctr;
> + hwh_dtr_fn dtr;
> +
> + hwh_pg_init_fn pg_init;
> + hwh_err_fn err;
> + hwh_status_fn status;
> +};

Please loose the prototypes, don't use prefixes/suffixes and use more 
descriptive names. Reasons are in Documentation/CodingStyle, Chapter 4.

So I suggest declaring it like this:

struct hardware_handler_operations {
 char *name;
 struct module *module;

 int (*create) (struct hw_handler *handler, unsigned int argc, char **argv);
 void (*destroy) (struct hw_handler *handler);

 void (*pg_init) (struct hw_handler *handler, unsigned int bypassed, 
                           struct path *path);

 unsigned (*error) (struct hw_handler *hwh, struct bio *bio);
 int (*status) (struct hw_handler *hwh, status_type_t type,  char *result, 
      unsigned int maxlen);
};

But you might want to loose status_type_t, too. Also hw_foo is a bit generic, 
isn't it? We are all dealing with "hardware" in any driver (which is 
basically another word for "hardware handler"). So please be a bit more 
creative on WHAT you drive.


Regards

Ingo Oeser