Skip to content

Conversation

mistydemeo
Copy link
Contributor

If term.h is present, use that instead of defining prototypes for the terminfo functions in terminfo-core.c. This causes problems on certain platforms (e.g. Apple aarch64) due to the functions being prototyped as non-variadic but called as variadic. If term.h isn't found, it falls back to the old behaviour.

This fixes #1238; I was able to test on my Apple ARM Mac and it builds/works as expected using the system curses. I wasn't able to test the meson build since it's failing for me for unrelated reasons, but I think my fix should fix that as well.

@ailin-nemui
Copy link
Contributor

thanks for the contribution! small nit pick, could you indent the snippet in the configure.ac so it is visually within the if

If term.h is present, use that instead of defining prototypes for the
terminfo functions in terminfo-core.c. This causes problems on certain
platforms (e.g. Apple aarch64) due to the functions being prototyped as
non-variadic but called as variadic. If term.h isn't found, it falls
back to the old behaviour.

Fixes irssi#1238.
@mistydemeo
Copy link
Contributor Author

Sure thing! Done.

@ailin-nemui
Copy link
Contributor

I tested it on Solaris, works as expected

@ailin-nemui ailin-nemui added the auto-merge This PR is scheduled for merge if no further comments are opened label Mar 28, 2021
@ailin-nemui
Copy link
Contributor

@irssi/developers

@ailin-nemui ailin-nemui merged commit a731525 into irssi:master Apr 1, 2021
@mistydemeo mistydemeo deleted the check_for_term_h branch April 2, 2021 05:27
ailin-nemui added a commit that referenced this pull request Apr 8, 2021
Add a check for term.h

(cherry picked from commit a731525)
@ailin-nemui ailin-nemui added this to the 1.2.3 milestone Apr 8, 2021
@carlocab
Copy link

carlocab commented Apr 13, 2021

Just to double-check: this landed in 1.2.3, right?

I wasn't sure because the tag doesn't show up in the merged commit. This would be good to know before merging Homebrew/homebrew-core#75094, which removes patches backported from this PR.

Thanks!

@mistydemeo
Copy link
Contributor Author

Yes, I see this in 1.2.3's source: https://github.com/irssi/irssi/blob/1.2.3/configure.ac#L372-L374 Looks like it made it into the generated configure too.

@carlocab
Copy link

Saw that too; just wanted to make sure I wasn't missing anything. Thanks, @mistydemeo.

@ailin-nemui ailin-nemui modified the milestones: 1.2.3, 1.3.0 Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge This PR is scheduled for merge if no further comments are opened
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text starts from the right hand side on ARM Mac
3 participants