Remove shared or move libkmod-list.c into shared

[stoeckmann]

Right now, depmod includes libkmod-internal.h to access kmod_list functions, which are supposed to be used within libkmod. They are not exposed through libkmod.h, i.e. they are not part of the public API.

This could be fixed by moving kmod_list.c as list.c into shared, getting rid of libkmod-internal.h usage in depmod. At first, I would have preferred this solution.

But when I looked into config: move config handling to shared/, I noticed that configuration handling uses error logging, accessing ctx for correct logging function etc. This would definitely drag kmod-specific logic into shared, which is currently very independent code.

If we move out the error logging, we would have to repeat the error handling in libkmod-config.c and depmod. Rather easy to do, just duplicated logic again.

Should we expose libkmod functionality to tools, just like we do with libkmod-internal.h in depmod right now, we would also get rid of duplicated instructions in kmod and libkmod, i.e. we would get a smaller binary.

So these are my proposals. Would like to get some feedback which one is preferable.

Option 1

1. Move code used in libkmod from shared into libkmod directory (array.c becomes kmodarray.c and all functions get `kmod` prefix)
2. Move code exclusively used in tools from shared into tools directory
3. Create a libkmod-shared.h file in libkmod
4. Include libkmod-shared.h in libkmod-internal.h
5. Include libkmod-shared.h in tools where needed

This will further reduce the binary size, since commonly used code actually exists only once as machine instructions. The kmod tools have more access into the library than other programs could (which makes them rather dependent on libkmod internals even if public API is stable).

Option 2

1. Move kmod_list.c from libkmod into shared (kmodlist.c becomes list.c and all functions lose their `kmod` prefix)
2. Create shared/list.h
3. Replace libkmod-internal.h in depmod with shared/list.h and libkmod.h

This keeps utility code separated from business logic. The kmod tools have same limitations and are as independent from libkmod internal changes as every other program which utilizes libkmod.

[lucasdemarchi]

Right now, depmod includes libkmod-internal.h to access kmod_list functions, which are supposed to be used within libkmod. They are not exposed through libkmod.h, i.e. they are not part of the public API.

it uses libkmod-internal to access additional things, like to have access to the struct definition, so the macros don't go through the @plt.

I think moving the list to shared and make a shim layer with kmod_ prefix in libkmod for things we export would be good.

[evelikov]

I don't think it's possible to remove shared/ without notable duplication - some parts are used across tools/, libkmod/ and `testsuite/".

IMHO it makes sense to flesh out parts which are used only in a single place (eg. pread_str_safe), which will allow us to remove the -{data,code}-sections which allegedly can lead to larger binaries. Although Lucas has raised concerns about this in the past.

Looking at the list alone - I would be in favour of having it in shared/.

[lucasdemarchi]

IMHO it makes sense to flesh out parts which are used only in a single place (eg. pread_str_safe), which will allow us to remove the -{data,code}-sections which allegedly can lead to larger binaries. Although Lucas has raised concerns about this in the past.

do you mean moving things out of shared? How does that reduce data/code? If we are building a binary/lib that doesn't use that function, that function will simply be discarded due to -ffunction-sections -fdata-sections

[evelikov]

The linker garbage collection -Wl,--gc-sections usually operates at section (in practise file) level. Eg. it will discard shared/array.c when the resulting binary does not reference anything from it.

By adding -ffunction-sections and -fdata-sections, each function and data respectively gets it's own ELF sub-section. Since the locality has changed the compiler may issue extra loads (depending on actual code, compiler, CPU arch, etc) and thus larger functions. At least according to some sources. It was never a priority so I haven't checked it personally - hence the allegedly from earlier.