Page MenuHomeFreeBSD

snd: simplify SND_DECLARE_FILE
AbandonedPublic

Authored by kbowling on Aug 16 2023, 6:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 31, 12:31 PM
Unknown Object (File)
May 2 2024, 5:30 AM
Unknown Object (File)
Apr 27 2024, 1:38 AM
Unknown Object (File)
Apr 26 2024, 1:28 AM
Unknown Object (File)
Apr 20 2024, 5:04 AM
Unknown Object (File)
Apr 10 2024, 7:54 PM
Unknown Object (File)
Feb 14 2024, 2:27 AM
Unknown Object (File)
Jan 3 2024, 10:38 AM
Subscribers

Details

Reviewers
imp
andrew
Summary

Follow up to 82a265ad9bad to remove string arg

The 'uniq' LINE gives more deterministic load order but seems a little sketchy.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

jrtc27 added inline comments.
sys/dev/sound/pcm/sound.h
362

The argument needs to be unique so that sndstat_unregisterfile can identify it. Other places do also use it as a string for printing purposes. Look for ->str in sndstat.c. This likely needs a bunch of reworking to not treat it as a version for the purposes of printing but rather to pass in an id for the name for the device like other driver-y things.

rpokala added inline comments.
sys/dev/sound/pcm/sound.h
355

Won't this break if multiple drivers happen to invoke this macro on the same line? Isn't that likely to happen, since it's usually early in the file, after the license and common headers? In fact, just scanning, there are definitely some cases of that happening.

Perhaps keep static char sndstat_vinfo[], and set the string to __FILE__? That would preserve the overall uniqueness of the original version, while removing the unnecessary macro arguments.

sys/dev/sound/pcm/sound.h
355

No; the SYSINIT uniquifier is only used to declare a static linker set member, so needs to be unique only within the translation unit. The version was never being used for that, and the uniq was never being used for sndstat_vinfo. The only issue is the one I highlighted below.

sys/dev/sound/pcm/sound.h
355

Yes, this is the sketchiness I referred to and no it does not regress versus what is currently going on, the string was not relevant to the situation. Using a line number as the 'uniq' is the issue.

sys/dev/sound/pcm/sound.h
355

Yes it does, and no it's not. Please go read sndstat.c very carefully and reread what I said above.

The diff you need here is much larger and also needs to be merged back to stable/13 as this has all been somewhat broken (but not fatally) since the switch to git.

These SYSINITs are used to build a linked-list of strings that are printed from one of the handlers. I've started working on a version that is MFC-safe that keeps the API but removes the implementation. I'll post it once it is ready. For HEAD we ultimately want to remove the macro entirely I believe.