Skip to content

Conversation

@NHOrus
Copy link

@NHOrus NHOrus commented Feb 11, 2025

Fixes errors that were warnings, once.

  1. Fixes system include that fails to provide argument to function and breaks all packages using it by that.
  2. Prefix gpm-specific functions with same name as function from curses with gpm_
  3. Fill function definition in yacc source with correct arguments, so compiler ceases screaming about three arguments to 0-argument function. Add 0 as third argument when it's two-argument function.

Copy link

@helmutg helmutg left a comment

Choose a reason for hiding this comment

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

Thanks for proposing fixes for gcc 15. This is a non-maintainer review and does not bear any authority.

This fails with GCC-15 and other compilers implementing C23
Added prefix gpm_ to raw() and noraw() used in two files because they
are implemented in same two files, and are static.
Same with scr_dump() and src_restore() in yacc source file
Fill empty argument lists with values. Add 0 as third argument for
two-argument function, it will be ignored at call site.
@NHOrus
Copy link
Author

NHOrus commented Feb 21, 2025

Ok, removed the ncurses include commit, leaving behind two that aren't problematic.

@helmutg
Copy link

helmutg commented Feb 21, 2025

Whilst this MR is now incomplete, the parts it fixes are an improvement. Thus I issue a non-maintainer LGTM with no authority.

Window handle is an opaque pointer that Gpm_Wgetch() passes
through straight to ncurses if it's not null and calls getch
if it's null. Code doesn't care what's inside the handle
as long as it agrees with curses definition.
@NHOrus
Copy link
Author

NHOrus commented Feb 21, 2025

I had a brilliant idea.
This builds with GCC-15 and should not conflict with ncurses.

* if it's null. Code doesn't care what's inside the handle
* as long as it agrees with curses definition.
*/
typedef struct _win_st WINDOW;
Copy link

Choose a reason for hiding this comment

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

I considered this as well, but it has two downsides.

If you also include curses.h, you have two typedefs and that doesn't make gcc happy.

It also exposes ncurses internals and breaks if ncurses ever renames its struct _win_st to something else.

Copy link
Author

Choose a reason for hiding this comment

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

And we can't split this declaration off into new header that's only optionally included, because ncurses straight-up checks for decl presence.

Can we ask ncurses to drop dependency on gpm? Or, you know, take the gpm and merge it into ncurses codebase...

Copy link

Choose a reason for hiding this comment

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

I guess we can take a void* here and cast it to WINDOW* in the .c file.

Choose a reason for hiding this comment

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

Can ncurses check for a different declaration?

Copy link

Choose a reason for hiding this comment

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

I guess we can take a void* here and cast it to WINDOW* in the .c file.

Yes, that's what I will upload to debian

algitbot pushed a commit to alpinelinux/aports that referenced this pull request Jul 29, 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.

5 participants