Symbol naming (libvgm prefix to reduce namespace conflicts)

[superctr]

We were discussing the naming of the various things in #58, and this made me think of something that actually has irked me for a little while.

I think that libvgm would definitely benefit if an effort is made to ensure that all exported symbols (and types) are in a libvgm namespace. Since C obviously doesn't have a namespace, this results to having to use a libvgm (or possibly an abbreviation like LVGM if it becomes too much to type) prefix in symbols and types.

Not only does it reduce the likelihood that you would accidentally use a libvgm-reserved symbol in a big project, it would also make it easy to identify functions that call the libvgm library.

The old library name vgm was perhaps a little too generic for this change to actually be useful, but with the renaming of files to libvgm, it would break backwards compatibility for a moment, and I think that would make it a good opportunity to make this (admittedly big) change.

[ValleyBell]

sounds like a good idea to me

EDIT: I think LVGM would be nice, as most functions are already prefixed.
Examples are AudioDrv_Start, SndEmu_Start, Resmpl_Init.

[jprjr]

I actually had this come up a while ago (and I mean a while ago, don't remember the exact symbol anymore). I had a build of MPD with libvgm and kode54's fork of gme, which includes an older copy of VGMPlay. Certain VGMs would cause the player to crash due to resolving some of the symbols into libgme instead of libvgm. It was one of the emulator cores that's in C.

I got around it by compiling with -Wl,-Bsymbolic -Wl,-Bsymbolic-function but that's probably not really portable to other operating systems/linkers.

It would be a pretty big undertaking to change all those symbols to be prefixed but I could help if you want to go that route. Basically change anything that isn't static to be prefixed with LVGM

[ValleyBell]

Most of the functions in the actual cores should actually be static by now - except for the FM cores, due to the way opnintf.c/oplintf.c work.

What is non-static in the emu library shoud be:

• all API functions (see SoundEmu.h, Resampler.h)
• the stuff in panning.h and dac_control.h ("dac_control" should really be renamed to... something else anyway)
• all device definition variables (devDefList_* and devDef_*)
• functions used by OPL and OPN FM cores

[jprjr]

global-emu-symbols.txt

all-emu-symbols.txt

Attaching a listing of global symbols, and a list of all symbols period from the emu library, for reference.

So to clarify - should all symbols be prefixed with LVGM for consistency? For example, here's the symbols from Resampler.c:

Resampler.c.o:
0000000000000269 T Resmpl_ChangeRate
0000000000000232 T Resmpl_Deinit
0000000000000000 T Resmpl_DevConnect
0000000000000afe t Resmpl_Exec_Copy
0000000000000c86 t Resmpl_Exec_LinearDown
0000000000000756 t Resmpl_Exec_LinearUp
0000000000000321 t Resmpl_Exec_Old
00000000000010a7 T Resmpl_Execute
00000000000000c3 T Resmpl_Init
000000000000007d T Resmpl_SetVals

If we just prefix the global symbols, there'd be a mis-mash of names like:

Resampler.c.o:
0000000000000269 T LVGM_Resmpl_ChangeRate
0000000000000232 T LVGM_Resmpl_Deinit
0000000000000000 T LVGM_Resmpl_DevConnect
0000000000000afe t Resmpl_Exec_Copy
0000000000000c86 t Resmpl_Exec_LinearDown
0000000000000756 t Resmpl_Exec_LinearUp
0000000000000321 t Resmpl_Exec_Old
00000000000010a7 T LVGM_Resmpl_Execute
00000000000000c3 T LVGM_Resmpl_Init
000000000000007d T LVGM_Resmpl_SetVals

That could get frustrating if you're working on something in that file, and have to context switch between a global vs local function. Or maybe that helps make it obvious about what's an external-facing function?

[ValleyBell]

I don't think that the Resmpl_Exec functions need a prefix. The mishmash looks weird indeed though.
Maybe they don't even need the Resmpl_ that they have right now.

Also, an additional thought: For Windows, I need to explicitly indicate DLL export functions. (which I don't do right now, but on Windows I always went with static build so far anyway)
So we probably should go with -fvisibility=hidden on Linux and just explicitly mark functions to be exported.

This doesn't solve the functions with same name" problem, but it hides most of the internal sound core functions that I can't make "static" due to how the OPL/OPN core selection works currently.

[superctr]

I don't think there will be a mishmash like that though, the symbols are sorted alphabetically. It will be pretty clear which symbols are exported and which ones are local.

[jprjr]

Re: providing a prefix for the Resmpl functions, I'm a big believer in libraries giving every function, enum, struct, typedef, etc a prefix. So, I'd lean towards still giving them the LVGM_ prefix, and doing it to all public, hidden, and local/static symbols. That way, even if something is exported that you didn't want to export - probably not a big since since it's prefixed anyway.

Doing the visibility flag certainly helps - but like you mentioned, it doesn't do anything for a static library, at least not without some post-build steps (you can do a partial link with ld to combine object files into one, then use objcopy to convert hidden symbols into local).

I would lean towards giving everything a prefix.