[PD-dev] definition/declaration of propertiesfunction

[Miller Puckette via Pd-dev]

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 at lists.iem.at
> http://lists.puredata.info/listinfo/pd-dev