Skip to content

Clean up ICU shim#2243

Merged
alerque merged 1 commit into
sile-typesetter:masterfrom
alerque:reduce-mallocs
Mar 20, 2025
Merged

Clean up ICU shim#2243
alerque merged 1 commit into
sile-typesetter:masterfrom
alerque:reduce-mallocs

Conversation

@alerque
Copy link
Copy Markdown
Member

@alerque alerque commented Mar 20, 2025

Long term this shim might go away, but first I have to figure out what it is doing and make sure it is doing it right so we even have something to compare against.

Closes #359

Copy link
Copy Markdown
Contributor

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

3-byte UTF-8 excludes non-BMP chars! (> U+FFFF )

Comment thread justenough/justenoughicu.c Outdated
Comment thread justenough/justenoughicu.c Outdated
@alerque alerque force-pushed the reduce-mallocs branch 2 times, most recently from 3006506 to c51d801 Compare March 20, 2025 18:07
@alerque
Copy link
Copy Markdown
Member Author

alerque commented Mar 20, 2025

Thanks for the obviously correct byte count @srl295.

What isn't obvious to me is whether the original issue your reported has been properly and completely fixed. The cheese has just moved too much for my lack of C knowledge. It does look like there is one less malloc() in that look and the writes to the Lua side make more sense to me, but still...

The original you reported the issue on would have been from just before the v0.9.4 release:
https://github.com/sile-typesetter/sile/blob/v0.9.4/src/justenoughicu.c#L74

If it is done it would be nice to know and get the issue closed with this final tweak.

@srl295
Copy link
Copy Markdown
Contributor

srl295 commented Mar 20, 2025

Thanks for the obviously correct byte count @srl295.

+1

What isn't obvious to me is whether the original issue your reported has been properly and completely fixed. The cheese has just moved too much for my lack of C knowledge. It does look like there is one less malloc() in that look and the writes to the Lua side make more sense to me, but still...

I replied on that issue that yes it does seem fixed.

The original you reported the issue on would have been from just before the v0.9.4 release: https://github.com/sile-typesetter/sile/blob/v0.9.4/src/justenoughicu.c#L74

If it is done it would be nice to know and get the issue closed with this final tweak.

@alerque alerque added this to the v0.15.10 milestone Mar 20, 2025
@alerque alerque marked this pull request as ready for review March 20, 2025 18:24
…onversion

Co-authored-by: Steven R. Loomis <srl295@gmail.com>
@alerque alerque merged commit e42b8de into sile-typesetter:master Mar 20, 2025
@alerque alerque deleted the reduce-mallocs branch March 20, 2025 18:25
@alerque alerque self-assigned this Mar 20, 2025
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.

unnecessary mallocs in justenoughicu.c

2 participants