hi,
my fellows from the debian qa team, notified me of another inconstistency between function definition, declaration and usage. the full bug-report can be found at [1].
the problem is with the properties-function (line numbers refer to today's master branch; but are similar in 0.45) declaration in g_canvas.c:1461 extern void canvas_properties(t_gobj *z); definition in g_editor.c:1045 void canvas_properties(t_glist *x); whereas it really should be (m_pd.h:504) void canvas_properties(t_gobj *x, t_glist *glist);
the complaint was, that (t_gobj) as used in the declaration is much wider than the (t_glist) as used in the function definition. (a t_glist is always a t_gobj, but not necessarily otherwise).
the omitting of the 2nd argument is probably less important (but for good style, we might add an unused argument...)
i think it should be clarified, what these arguments are really meant to be. i guess the properties-callback really gets a pointer to the object itself, and a pointer to the canvas where the object lives in.
the easy part is probably the second argument, which i guess should be a "t_canvas*" rather than a "t_glist*", although both are the same (#defined ... struct _glist).
i'm not entire sure about the 1st argument, though. what i would like to have is: - have all per-object callbacks use the same type - document that this pointer can indeed by cast to the object-instance pointer
afaict, the types used for callbacks are currently: - (t_pd*) used for all message-callbacks to objects (and also returned by pd_new() which is used to construct an object-instance. - (t_gobj*) used for all graphics-callbacks
a noteable exceptions is (t_savefn) which uses (t_gobj*) but probably should use (t_pd*) instead, as the savefn isn't bound to graphical objects (though it's probably needed more for non-text-editable objects)
i haven't found any documentation (in the header-files) that states that the first callback argument is indeed what we expect it to be (a pointer to the called back object).
anyhow, what is the proper fix for the inconsistent canvas_properties()? i assume that it really ought to be void canvas_properties(t_gobj*z, t_glist*canvas) { t_glist*x=(t_glist*)z; // ... }
i've submitted a patch at sourceforge [525].
gfadsrm IOhannes
[1] https://bugs.debian.org/751061 [525] https://sourceforge.net/p/pure-data/patches/525/
Hi all -
Hannes's patch is a good fix for the immediate problem.
My own thought is similar to his - t_pd for any callback requiring only t_pd behavior, and t_gob for ones that can be items on canvases and might need to use a back-pointer to the owning canvas.
I don't know which of these "savefn" should be - it's never called for anything except a gobj, yet nobody (yet) seems to have needed to access the parent glist to save itself. I can't guarantee that won't happen in the future though, nor that nobody out in Pd extended land is using that (which would then break if it got changed). So lacking any urgent reason to change it I'm thinking it's best left alone.
cheers Miller
On Wed, Jun 25, 2014 at 11:58:05AM +0200, IOhannes m zmölnig via Pd-dev wrote:
hi,
my fellows from the debian qa team, notified me of another inconstistency between function definition, declaration and usage. the full bug-report can be found at [1].
the problem is with the properties-function (line numbers refer to today's master branch; but are similar in 0.45) declaration in g_canvas.c:1461 extern void canvas_properties(t_gobj *z); definition in g_editor.c:1045 void canvas_properties(t_glist *x); whereas it really should be (m_pd.h:504) void canvas_properties(t_gobj *x, t_glist *glist);
the complaint was, that (t_gobj) as used in the declaration is much wider than the (t_glist) as used in the function definition. (a t_glist is always a t_gobj, but not necessarily otherwise).
the omitting of the 2nd argument is probably less important (but for good style, we might add an unused argument...)
i think it should be clarified, what these arguments are really meant to be. i guess the properties-callback really gets a pointer to the object itself, and a pointer to the canvas where the object lives in.
the easy part is probably the second argument, which i guess should be a "t_canvas*" rather than a "t_glist*", although both are the same (#defined ... struct _glist).
i'm not entire sure about the 1st argument, though. what i would like to have is:
- have all per-object callbacks use the same type
- document that this pointer can indeed by cast to the object-instance
pointer
afaict, the types used for callbacks are currently:
- (t_pd*) used for all message-callbacks to objects (and also returned
by pd_new() which is used to construct an object-instance.
- (t_gobj*) used for all graphics-callbacks
a noteable exceptions is (t_savefn) which uses (t_gobj*) but probably should use (t_pd*) instead, as the savefn isn't bound to graphical objects (though it's probably needed more for non-text-editable objects)
i haven't found any documentation (in the header-files) that states that the first callback argument is indeed what we expect it to be (a pointer to the called back object).
anyhow, what is the proper fix for the inconsistent canvas_properties()? i assume that it really ought to be void canvas_properties(t_gobj*z, t_glist*canvas) { t_glist*x=(t_glist*)z; // ... }
i've submitted a patch at sourceforge [525].
gfadsrm IOhannes
[1] https://bugs.debian.org/751061 [525] https://sourceforge.net/p/pure-data/patches/525/
Pd-dev mailing list Pd-dev@lists.iem.at http://lists.puredata.info/listinfo/pd-dev
-
IOhannes m zmölnig via Pd-dev -
Miller Puckette via Pd-dev