On Fri, 25 Mar 2005, Pavel Machek wrote:
> In 2.6.11, we had:
>
> static int foo_suspend(struct pci_dev *pdev, u32 state)
>
> . that's wrong. Now we have
>
> static int foo_suspend(struct pci_dev *pdev, pm_message_t state)
>
> , which is slightly better, but people still get it wrong, because
> pm_message_t is compatible with u32. Oops. Obvious solution is to make
> pm_message_t typedefed into struct, so people can't do the typing
> wrong. This is kernel 101.
>
> What you would like to have is
>
> static int foo_suspend(struct pci_dev *pdev, struct pm_message *state)
>
> which I agree is marginally nicer to look at, but still does not
> provide enough typechecking and [more importantly] there's no way in
> hell we are doing second search and replace over all the drivers.
Are you willing to go halfway? If we do something more like this:
typedef struct {
struct pm_message *m;
} pm_message_t;
then the code can pass pm_message_t's around with full type-checking and
still retain the benefit of sending a pointer rather than a structure.
Yes, it looks stupid. But it's a reasonable compromise that won't require
changing the prototypes of a million drivers. (You could make it look a
little less stupid by changing the name of struct pm_message -- a minor
detail.)
It _will_ require auditing for drivers that store the message as their
state, which is maybe just about as hard. However almost everyone agrees
that using a message to represent a state is a bad thing to do. The
simplest way to fix it would be to store message.m->event as the state
instead of message itself.
Alan Stern
