Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent double-precision externals from crashing a single-precision Pd (and vice versa) #300

Merged
merged 12 commits into from Sep 9, 2018

Conversation

umlaeute
Copy link
Contributor

this implements a simple, API-compatible mechanism to prevent double-precision externals from being loaded into a single-precision Pd and vice versa, as proposed on the mailnglist and mentioned in #299

how it works

  • single-precision (PD_FLOATTYPE==32) externals use class_new() to register new classes
  • double-precision (PD_FLOATTYPE==64) externals use class_new64() to register new classes

if an external uses the wrong variant of class_new (e.g. class_new64() on a single-precision Pd host) will yield an error message and the class will not be registered (with class_new returning NULL).

API compatibility is kept by using a pre-processor define, so externals keep using class_new in their code, but it gets magically replaced with class_new64, if PD_FLOATTYPE==64.

ABI (binary) compatibility is retained for single-precision externals (since we still use class_new()), but not for double-precision externals.

this also adds some NULL-pointer checks to functions with a t_class* argument, since practically all externals do not check whether class_new returns NULL.

@umlaeute umlaeute added the feature suggestion for an enhancement label Jan 31, 2018
Copy link
Contributor Author

@umlaeute umlaeute left a comment

Choose a reason for hiding this comment

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

the first commit (by @lukexi) is slightly unrelated to double precision, but since double is also a lot about "64bit" I included it :-)

@umlaeute umlaeute added this to the 0.49 milestone Sep 6, 2018
@umlaeute
Copy link
Contributor Author

umlaeute commented Sep 6, 2018

up to date with today's (0.49-0test0) master branch

@millerpuckette
Copy link
Contributor

I think this can be done better by having 64-bit-float versions of Pd call xxx_setup64(). That way,
a single object file could have both 32 and 64 bit code.

@umlaeute
Copy link
Contributor Author

umlaeute commented Sep 9, 2018

the proposed way does allow single binaries to have both 32 and 64 bit code, as the external might still call class_new64() to register double-precision objects even when compiled with PDFLOATSIZE=32 (and vice-versa; although in the latter case it has to undefine the class_new override first. but that's pretty simple).

it also caters for libraries that do not use the xxx_setup function at all, but instead call the class_new() from _init(), __attribute__((constructor)) or a static C++-object.
at least Gem uses the last option to register it's classes (without having to enumerate them; in the light of 200+ objects, it seemed like a clever idea back then...)

and you also need to cater for hexmangled _setup function, which I think would add even more confusion to the actual name of the setup-function.

finally, it requires code-changes for each and every library to support double-precision builds.
in contrast, my proposal should allow to compile any (well-written) external without any code-changes and a single preprocessor define (or a one-line change in the included Pd-header, if injecting preprocessor defines is unfeasible)

@umlaeute
Copy link
Contributor Author

umlaeute commented Sep 9, 2018

however, there is one drawback in my proposal: an external that is compiled for double-precision (either as "only double-precision" or "single + double-precision") will not be loadable by a legacy Pd.

@millerpuckette millerpuckette merged commit 8a259db into master Sep 9, 2018
@umlaeute umlaeute deleted the float64 branch September 10, 2018 05:40
umlaeute added a commit to pure-data/deken that referenced this pull request Feb 1, 2019
umlaeute added a commit that referenced this pull request Dec 3, 2019
… to load a single-precision external, or vice versa

this was somehow stripped from my original PR.
this commit is a new (and I hope somewhat simpler attempt) to solve the issue

Related: #300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature suggestion for an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants