[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