Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libfdt: Add symbols to their corresponding versions #108

Closed
wants to merge 1 commit into from
Closed

libfdt: Add symbols to their corresponding versions #108

wants to merge 1 commit into from

Conversation

ernestask
Copy link

@ernestask ernestask commented Jul 28, 2023

787b599 added the linker script with symbol versions, but, over time, new versions were not added, making versioning somewhat pointless. This commit aims to put symbols under versions they were introduced in. As a result, some previously-local symbols also become global.

libfdt/version.lds Outdated Show resolved Hide resolved
@ernestask
Copy link
Author

Gah, I put them under the wrong version.

787b599 added the linker script with
symbol versions, but, over time, new versions were not added, making
versioning somewhat pointless. This commit aims to put symbols under
versions they were introduced in. As a result, some previously-local
symbols also become global.

Signed-off-by: Ernestas Kulik <ernestas@baltic.engineering>
@ernestask ernestask changed the title libfdt: Make fdt_get_symbol{_namelen}() public libfdt: Add symbols to their corresponding versions Jul 29, 2023
@ernestask
Copy link
Author

I’ve changed the topic of this PR and shuffled symbols around to match the versions they were introduced in, as now all of them will appear under LIBFDT_1.2. Wasn’t sure what to do with unreleased ones, so I put them under a fictitious 1.8 version.

@dgibson
Copy link
Owner

dgibson commented Aug 1, 2023

787b599 added the linker script with symbol versions, but, over time, new versions were not added, making versioning somewhat pointless.

Uh... yes and no. So, my understanding is that adding new symbols to an existing version is more or less safe. It's perhaps not the ideal way of doing things, but it will work: old clients expecting the old version obviously won't be using the new symbols, but newer programs expecting the newer versions will find them. We only break the symbol versioning if we change the signature for a symbol without adding a new symbol version.

This commit aims to put symbols under versions they were introduced in. As a result, some previously-local symbols also become global.

While it probably would have been neater if we'd done it this way all along, I think changing this now could break existing binaries using the shared library: a binary could have been built with a recentish libfdt (but before this change) so it is expecting to see a new symbol (say fdt_overlay_target_offset()) within the LIBFDT_1.2 version. That would then break when it tries to run against a shared library with this change, where that symbol has been moved to a newer version.

So while it would be reasonable to always add new symbols to new versions in future, I don't think we can move already versioned symbols around.

fdt_stringlist_count;
fdt_stringlist_search;
fdt_stringlist_get;
fdt_resize;
fdt_get_max_phandle;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This symbol was changed into a header file inline in 1.6.0

@ernestask ernestask closed this Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants