Page MenuHomeFreeBSD

sound: Clean up pcm/ includes
Needs ReviewPublic

Authored by christos on Mon, Dec 2, 5:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 6, 11:31 AM
Unknown Object (File)
Wed, Dec 4, 9:51 PM
Unknown Object (File)
Wed, Dec 4, 9:51 PM
Unknown Object (File)
Wed, Dec 4, 9:29 PM
Unknown Object (File)
Wed, Dec 4, 7:14 AM
Subscribers

Details

Summary
  • Group pcm/ header files in pcm/sound.h instead of repeating in multiple files.
  • Re-organize sections in pcm/sound.h and sort alphabetically.
  • Get rid of unnecessary includes.
  • Include opt_snd.h once in pcm/sound.h, instead of every single file.
  • Move all includes at the top of each file.

Sponsored by: The FreeBSD Foundation
MFC after: 2 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60918
Build 57802: arc lint + arc unit

Event Timeline

Not really sure what to do with SND_USE_FXDIV and SND_DECLARE_FXDIV. Also I will write some follow-up patches to clean up the drivers as well.

I have to admit the centralization through sound.h goes against my intuition - usually I would only include what is needed, where it's actually used. While this change reduces the number of includes, it makes it even harder to see the dependencies and layers in the sound module. But that's just my opinion, I suppose @markj and @emaste may be the better judges in this case.

I have to admit the centralization through sound.h goes against my intuition - usually I would only include what is needed, where it's actually used. While this change reduces the number of includes, it makes it even harder to see the dependencies and layers in the sound module. But that's just my opinion, I suppose @markj and @emaste may be the better judges in this case.

While I kind of agree, I notice that some of the includes are simply repeated over and over, so IMHO it's better to just have them in one place. After all, we include pcm/sound.h everywhere anyway. Regarding dependencies and layering, I intentionally excluded pcm/sndstat.c-specific includes since they are not needed anywhere else, as well the foo_if.h ones, which I kept/moved to their respective header files.

An alternative proposal is that we strip pcm/sound.h of any unnecessary includes, and instead do what you said: include only what is needed, where it's needed.

I have to admit the centralization through sound.h goes against my intuition - usually I would only include what is needed, where it's actually used. While this change reduces the number of includes, it makes it even harder to see the dependencies and layers in the sound module. But that's just my opinion, I suppose @markj and @emaste may be the better judges in this case.

While I kind of agree, I notice that some of the includes are simply repeated over and over, so IMHO it's better to just have them in one place. After all, we include pcm/sound.h everywhere anyway. Regarding dependencies and layering, I intentionally excluded pcm/sndstat.c-specific includes since they are not needed anywhere else, as well the foo_if.h ones, which I kept/moved to their respective header files.

Regarding foo_if.h, since the foo.h include them now they are also transitively included in sound.h, and thus everywhere. I agree we already include a lot of things in most places, so it's probably not a big issue. But I'd like to know about FreeBSD kernel best practice.