Portmidi header change breaks old code

[Starbuck5]

Hello, I work on maintaining pygame-ce, which uses portmidi for the pygame.midi module

We've recently seen compiles start to fail in our autogenerated pypm.c

src_c/pypm.c:7236:81: error: incompatible function pointer types passing 'PtTimestamp (*)(void)' (aka 'int (*)(void)') to parameter of type 'PmTimeProcPtr' (aka 'int (*)(void *)') [-Wincompatible-function-pointer-types]
 7236 |   __pyx_v_err = Pm_OpenInput((&__pyx_v_self->midi), __pyx_t_2, NULL, __pyx_t_3, (&Pt_Time), NULL);
      |                                                                                 ^~~~~~~~~~
/data/data/com.termux/files/usr/include/portmidi.h:399:31: note: passing argument to parameter 'time_proc' here
  399 |                 PmTimeProcPtr time_proc,
      |                               ^
1 error generated.

From my research, it looks like the function Pt_Time used to be compatible with PmTimeProcPtr, but now it is not.

I think this broke here: https://github.com/PortMidi/portmidi/commit/64314cc3d1a6fdddfc6ff5408a3f83af685b8cea#diff-a92ece8a1b564c101a59ed27031b813d53906570f99c4f12bd09ac5f3b65fd82L79-R84

Apparently, in C, a function declaration with no args does not mean the function has no args-- https://stackoverflow.com/a/5929736/13816541. So Pt_Time() and Pt_Time(void) are different.

We have a PR to insert a shim to make Pt_Time used to be compatible with PmTimeProcPtr on any version of portmidi, but I wanted to send in this bug report as an FYI.

[rbdannenberg]

I don't see the PR - maybe that was to fix pypm.c? One way to handle this is to cast Pt_Time to PmTimeProcPtr if you want to use it as a PmTimeProcPtr. Pt_Time does not require a parameter. I'm not too clear on whether this is still correct C or it just works (it was certainly correct on earlier C compilers). But, it's certainly not the cleanest approach. Changing the type signature of Pt_Time is really an incompatible change, but I think it's the right thing to do. It means any direct call to Pt_Time should change to Pt_Time(NULL).

[Starbuck5]

Here's the PR on our end if you're curious about our fix: https://github.com/pygame-community/pygame-ce/pull/2863

One way to handle this is to cast Pt_Time to PmTimeProcPtr if you want to use it as a PmTimeProcPtr

This was my first instinct too, but I couldn't figure out how to generate that C code out of Cython (the Python-esque language that generates into pypm.c)

[oddbookworm]

We still get an error when trying to case Pt_Time to PmTimeProcPtr because the typedef of PmTimeProcPtr says it needs a void* argument

https://github.com/PortMidi/portmidi/blob/master/pm_common/portmidi.h#L339

[rbdannenberg]

You tried (PmTimeProcPtr) &Pt_Time?

[oddbookworm]

Yes, here's exactly what the code looks like:

        err = Pm_OpenInput(&(self.midi), input_device, NULL, buffersize,
                           <PmTimeProcPtr>&Pt_Time, NULL)

And the compiler error

      Error compiling Cython file:
      ------------------------------------------------------------
      ...
              cdef PmError err
              self.device = input_device
              self.debug = 0

              err = Pm_OpenInput(&(self.midi), input_device, NULL, buffersize,
                                 <PmTimeProcPtr>&Pt_Time, NULL)
                                 ^
      ------------------------------------------------------------

      C:\Users\andre\Projects\pygame-ce\src_c\cython\pygame\pypm.pyx:545:27: Cannot assign type 'PmTimeProcPtr' (alias of 'PmTimestamp (*)(void *) noexcept') to 'long (*)(void) noexcept'

[rbdannenberg]

This is not C or C++. I guess you are saying the solution for C programs: (PmTimeProcPtr) &PtTime does not have an equivalent in Cython.

[oddbookworm]

That is the Cython equivalent, the <PmTimeProcPtr>&Pt_Time

[oddbookworm]

I'll try to manually compile the cython without the cast, then do the C-style cast in the C manually to see what happens

[oddbookworm]

Ok, yeah, manually casting it in the generated C does fix the problem. I've opened an issue with cython to report a potential bug in casting here

[rbdannenberg]

Thanks for doing the test. Although it does sound like cython should work differently, I now also think Pt_Time should match PmTimeProcPtr and I should change all PmTimeProcPtr to PtTimeProcPtr so that the type of the time function will be declared in porttime.h. That will eliminate some casts, enable more type checking, but probably break something else. It shouldn't affect any binaries though.