Use int64_t as fluid_long_long_t in C99

[devingryu]

Currently, fixed type of long long is used for fluid_long_long_t, as it is guranteed to have 64-bit wide in C99.
However, stdint.h defines int64_t as long, so it has posibillity to break codes which uses 64-bit type with stdint.
For example, jni.h defines jlong with int64_t, so some functions which uses fluid_long_long_t as parameter type breaks when binding them into jlong.

So, even though using long long as 64-bit type is reasonable, I suggest you to use int64_t since it has potential to break in some build targets.

Thanks in advance.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[derselbst]

For example, jni.h defines jlong with int64_t, so some functions which uses fluid_long_long_t as parameter type breaks when binding them into jlong.

Sry, I don't get this point. Why would you want to bind fluid_long_long_t to jlong?

An why would this break something? You're essentially defining fluid_long_long_t == jlong == int64_t == long. The only chance I see this would go wrong is when fluidsynth was compiled on a machine where long long is actually an 128 bit integer, whereas long is only 64 bit.

You're basically suggesting an ABI breakage for everyone. So I really need to understand your motivation.

[devingryu]

My situation is related to auto generation of binding code.
In 64-bit target, int64_t==jlong==long, and fluid_long_long_t==long long.
When calling functions (or setting callback functions) which includes fluid_long_long_t, the value of java long(which came from java) should be explicitly type converted in order to be passed to fluidsynth C functions.

But come to think of it, other programs using fluidsynth could produce build error if the type of fluid_long_long_t is changed (though the build process of the library seems to go well). The issue I made is small compared to that, so you may ignore this and close. I can just make a hardcoded version of jni code and use it.

[derselbst]

When calling functions (or setting callback functions) which includes fluid_long_long_t, the value of java long(which came from java) should be explicitly type converted in order to be passed to fluidsynth C functions.

It's pretty uncommon to mess around in 3rd party library headers. Have you considered to typecast fluid_long_long_t to int_least64_t? If so, what does your jni.h say about this type?

But come to think of it, other programs using fluidsynth could produce build error if the type of fluid_long_long_t is changed (though the build process of the library seems to go well)

I do breaking change releases of fluidsynth from time to time. The next breaking change will probably be when porting to C++ (in case that ever happens, see #847). I will consider your request if or once we do so.

[devingryu]

It's pretty uncommon to mess around in 3rd party library headers. Have you considered to typecast fluid_long_long_t to int_least64_t? If so, what does your jni.h say about this type?

It differs per target platform, as it refers to stdint.h. in armv7-eabi and x86 it is not auto casted, but in aarch64 and amd64, it works well(since both int64_t and jlong is long)

The point of my opinion is not in jni.h, but in stdint.h. The best choice of 64-bit type varies per platform, so my point is it would be better to choose type of 64-bit type with stdint.h. So for portability and clarity, I suggest you to change fluid_long_long_t to int64_t (or int_fast64_t) when next breaking change comes. Since stdint.h is standard both in C99 and C++, it would increase portability for other compilers and stability would be guranteed when other architectures with 128-bit based comes up.

However, this is just a future talk, and I realized that the cost of the change is larger than maintaining the current situation. So I also think it is not that should be considered now. But when a breaking change happens, I hope that you could consider this.

I do breaking change releases of fluidsynth from time to time. The next breaking change will probably be when porting to C++ (in case that ever happens, see #847). I will consider your request if or once we do so.

Glad to hear from you! I'm looking forward to it.

[derselbst]

Ok thanks, I've added it to the discussion so we don't forget about it.

[devingryu]

OK, Thanks.