Menu ▾ ▴

#1145 inconsitent type of argc in A_GIMME-callback

v0.45
closed-accepted
Miller Puckette
API (1) devel (1)
5
2016-06-20
2014-06-02
IOhannes m zmölnig
No

recently, i received a bug-report for the "puredata" Debian-package,
that warned of conflicting declarations of the glist_scalar() function.

the problem seems to be, that glist_scalar() uses int argc for it's
A_GIMME argument in the declaration in g_canvas.c, but uses t_int argc in the function definition in g_scalar.c.
now t_int is defined as a "pointer-sized integer", which is a 64bit type on amd64, thus incompatible with the 32bit int.

it is often tempting to use t_int whenever possible, as it is the
Pd-native integer type. unfortunately this can (and will) break binary compatibility in some cases.

i think it should be well documented which types to use.
e.g. the only way to find out how to use A_GIMME is reading the source
code (where we have at least 4 occurences that use t_int
(glist_scalar(), curve_new(), plot_new() and drawnumber_new()) and a
lot more (>200) uses of int.

also t_listmethod and t_anymethod in m_imp.h seem to suggest that
int is the correct type (though m_imp.h was historically
considered a "private" header, not meant for documentation).

i think it's necessary to fix all uses of t_int argc to int argc.

additionally i think it would be good to:

  • clearly document the function-signature for (A_GIMME) callbacks (and
    while doing so, move the various callback-typedefs from m_imp.h to m_pd.h)

  • split the generic t_int type into multiple names, that suggest
    their usage.
    afaict, the original use of t_int is for the dsp process function
    (t_perfroutine), though the name does not suggest anything like this.
    probably we should move to a new (probably even more pointer-like) type
    typedef t_perfarg void*;
    and replace all other uses of t_int (e.g. the return value of
    atom_getint() or the struct-member t_resample.upsample) by other
    types, e.g. int,
    for compatibility reasons, we might need to keep the two (or more)
    types the same size, but they should be separated on a semantic level.

i'm attaching a patch that changes the use of t_int to int in A_GIMME callbacks.

1 Attachments
0001-use-int-argc-for-A_GIMME-callbacks.patch

Discussion

  • IOhannes m zmölnig

    IOhannes m zmölnig - 2014-06-02
    • Description has changed:

    Diff:

    --- old
+++ new
@@ -1,45 +1,42 @@
 recently, i received a bug-report for the "puredata" Debian-package,
-that warned of conflicting declarations of the glist_scalar() function.
+that warned of conflicting declarations of the `glist_scalar()` function.

-the problem seems to be, that glist_scalar() uses "int argc" for it's
-A_GIMME argument in the declaration in g_canvas.c, but uses "t_int
-argc" in the function definition in g_scalar.c.
-now "t_int" is defined as a "pointer-sized integer", which is a "long
-int" on amd64, thus incompatible with the 32bit "int".
+the problem seems to be, that `glist_scalar()` uses `int argc` for it's
+`A_GIMME` argument in the declaration in *g_canvas.c*, but uses `t_int
+argc` in the function definition in *g_scalar.c*.
+now `t_int` is defined as a "pointer-sized integer", which is a 64bit type on amd64, thus incompatible with the 32bit `int`.

-it is often tempting to use "t_int" whenever possible, as it is the
-Pd-native integer type.
-unfortunately this can (and will) break binary compatibility in some
-cases.
+it is often tempting to use `t_int` whenever possible, as it is the
+Pd-native integer type. unfortunately this can (and will) break binary compatibility in some cases.

 i think it should be *well documented* which types to use.
-e.g. the only way to find out how to use A_GIMME is reading the source
-code (where we have at least 4 occurences that use "t_int"
-(glist_scalar(), curve_new(), plot_new() and drawnumber_new()) and a
-lot more (>200) uses of "int".
+e.g. the only way to find out how to use `A_GIMME` is reading the source
+code (where we have at least 4 occurences that use `t_int`
+(`glist_scalar()`, `curve_new()`, `plot_new()` and `drawnumber_new()`) and a
+lot more (>200) uses of `int`.

-also t_listmethod and t_anymethod in m_imp.h seem to suggest that
-"int" is the correct type (though "m_imp.h" was historically
+also `t_listmethod` and `t_anymethod` in *m_imp.h* seem to suggest that
+`int` is the correct type (though *m_imp.h* was historically
 considered a "private" header, not meant for documentation).

-i think it's necessary to fix all uses of "t_int argc" to "int argc".
+i think it's necessary to fix all uses of `t_int argc` to `int argc`.

 additionally i think it would be good to:
-- clearly document the function-signature for (A_GIMME) callbacks (and
-while doing so, move the various callback-typedefs from m_imp.h to m_pd.h)

-- split the generic "t_int" type into multiple names, that suggest
+- clearly document the function-signature for (`A_GIMME`) callbacks (and
+while doing so, move the various callback-typedefs from *m_imp.h* to *m_pd.h*)
+
+- split the generic `t_int` type into multiple names, that suggest
 their usage.
-afaict, the original use of "t_int" is for the dsp process function
-(t_perfroutine) , though the name does not suggest anything like this.
+afaict, the original use of `t_int` is for the dsp process function
+(`t_perfroutine`), though the name does not suggest anything like this.
 probably we should move to a new (probably even more pointer-like) type

- typedef t_perfarg void*;
-and replace all other uses of t_int (e.g. the return value of
-"atom_getint()" or the struct-member "t_resample.upsample") by other
-types, e.g. "int",
+ `typedef t_perfarg void*;`
+and replace all other uses of `t_int` (e.g. the return value of
+`atom_getint()` or the struct-member `t_resample.upsample`) by other
+types, e.g. `int`,
 for compatibility reasons, we might need to keep the two (or more)
 types the same size, but they should be separated on a semantic level.


-
-i'm attaching a patch that changes the use of `t_int` to `int` in A_GIMME callbacks.
+i'm attaching a patch that changes the use of `t_int` to `int` in `A_GIMME` callbacks.
     

  • IOhannes m zmölnig

    IOhannes m zmölnig - 2014-06-03

    fixed with commit 8790e19930

     

  • IOhannes m zmölnig

    IOhannes m zmölnig - 2014-06-03
    • status: open --> pending-accepted
     

  • IOhannes m zmölnig

    IOhannes m zmölnig - 2016-06-20
    • status: pending-accepted --> closed-accepted
     

Anonymous
Anonymous

Add attachments
Cancel





MongoDB Logo MongoDB