[PATCH] staging: goldfish: Retire a mutable global variable
Roman Kiryanov
rkir at google.com
Mon Jul 16 18:09:48 UTC 2018
> > struct goldfish_audio {
> > + struct miscdevice miscdevice;
>
> Odd. misc devices are normally static to the file, you only need one of
> them, right?
Yes, I want one. I did not know that are expected to be static to the
file. I don't see why.
> Do you really have more than one audio device
> in the system at a time?
No. But if we happen to have more, this approach will create two
separate minors. Which is what we want.
I don't see advantages of the approach currently checked in.
> When you do this, you end up with crazy stuff like:
>
> > -static struct miscdevice goldfish_audio_device = {
> > - .minor = MISC_DYNAMIC_MINOR,
> > - .name = "eac",
> > - .fops = &goldfish_audio_fops,
> > +/* init miscdevice initial state to use with misc_register */
> > +static void init_miscdevice(struct miscdevice *misc)
> > +{
> > + misc->minor = MISC_DYNAMIC_MINOR;
> > + misc->name = "eac";
> > + misc->fops = &goldfish_audio_fops;
> > };
>
> that. Which is sad
I don't feel it is more crazy that it is now.
> (also are you sure the other fields are set to 0?
> I didn't look at the whole code path here.
miscdevice resides in goldfish_audio which is allocated by devm_kzalloc.
> What benifit is there to do this change?
I believe getting rid of global mutable variables is a benefit itself.
But I also see
static struct platform_driver goldfish_audio_driver = { ...
which is also global and mutable. I suppose it is ok in Linux.
I can drop this change. What else do I need to cleanup in
goldfish_audio to proceed with adding other drivers?
More information about the devel
mailing list