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

Implementing --clean-strtab #461

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

brenoguim
Copy link
Collaborator

@brenoguim brenoguim commented Feb 19, 2023

Hi,

I started to implement a simple version of --clean-strtab as suggested in #449.

Some attention points:

  • I'm not sure I got all the points.
  • This is really clean-dynstr so might need work to expand to other sections. But seems to be a good start for later enhancement.
  • Unfortunately, for most cases --clean-strtab will actually introduce a few strings because it will undo sharing like vprintf with printf. However I still think it's valuable to have it and enhance it later on if necessary.

Maybe we should have a set of switches documented as beta due to their complexity? The name renaming and this one could be marked as experimental because it's hard to know I got all fields right.
Then after a couple of releases they could be moved to stable.

@brenoguim brenoguim force-pushed the breno.str branch 5 times, most recently from 6bfd015 to 01cde37 Compare February 20, 2023 00:31
@brenoguim brenoguim changed the title WIP: Implementing --clean-strtab Implementing --clean-strtab Feb 20, 2023
@brenoguim brenoguim force-pushed the breno.str branch 7 times, most recently from 7e39024 to da7a592 Compare February 24, 2023 16:18
@rohoog
Copy link

rohoog commented Mar 2, 2023

I see that this pull request also does what I've done in #472. However, I don't introduce new symbols due to the symbol sharing, I just leave the sharing as-is.

@brenoguim
Copy link
Collaborator Author

I was planning to enhance it to do the sharing eventually, so it would be next to perfect.
I didn't check the size of the file in my test, only the presence of a given string.
I'll take a look at your PR @rohoog , maybe the approach of not undoing the sharing is the best.

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.

2 participants