Hi Hez,
Thanks for pointing this one out. Sorry I've not responded sooner, but
I've been away. I suppose ideally plcont should do the same thing as
plshades. Unfortunately for historical reasons plcont and plshades take
different arguments. Since we don't know the max / min range for the
axis in the plcont call then we can just use the trick plshades uses to
put the values linearly in the range xmin ... xmax and ymin ... ymax.
Calling pltr0 is not quite the same thing, and might in fact be more
confusing since it is different to plshades.
For this reason I prefer not applying option b. However option a and c
combined will prevent the segfault by checking pltr is not NULL and will
also correctly document the behaviour.
I've commited these changes with just a couple of modifications. Note
that in C source files we use C style comments like /* */ and not
C++ comments like //. Some examples of the latter have crept into the
source, but it is best avoided if possible. There was a time when not
all C compilers supported C++ comments, but I suspect those days are
gone.
Andrew
On Thu, Apr 03, 2008 at 02:35:33PM -0400, Hezekiah M. Carty wrote:
> I ran in to a problem with plcont while translating example 16 to
> OCaml. While writing the OCaml PLplot bindings I mistakenly assumed
> that the plcont function would accept NULL for its pltr and pltr_data
> arguments and fall back on a default transform of some sort. However,
> setting pltr to NULL gave me segmentation faults as plcont uses the
> pltr callback function regardless of its value.
>
> I have attached three small patches to this email in the hope that one
> of them will be acceptable to the core developers:
>
> plcont_doc_pltr.diff - This patch adds a note to the plcont
> documentation (api.xml) stating that behavior is undefined when pltr
> is NULL.
>
> plcont_default_pltr.diff - This patch changes plcont to use pltr0 as a
> callback when the pltr argument is NULL.
>
> plcont_pltr_null_abort.diff - This patch changes plcont to abort using
> plabort with an error if the pltr argument is NULL. This is my
> personal preference for a solution.
>
> While the documentation does not say that the pltr callback can be set
> to NULL for plcont, that is how (I think) all of the other plotting
> functions which accept a transformation callback work. Any of the
> above 3 patches will do something to remedy this problem. I
> personally prefer the 3rd patch (plcont_pltr_null_abort.diff) because
> it is less likely to surprise users with a potentially strange default
> transform and it removes a source of potential segmentation faults or
> other crashes.
>
> Sincerely,
> Hez
>
> --
> Hezekiah M. Carty
> Graduate Research Assistant
> University of Maryland
> Department of Atmospheric and Oceanic Science
> diff --git a/doc/docbook/src/api.xml b/doc/docbook/src/api.xml
> index 517e0a5..3815f9b 100644
> --- a/doc/docbook/src/api.xml
> +++ b/doc/docbook/src/api.xml
> @@ -1796,6 +1796,13 @@ from relative device coordinates,
> <xref linkend="contour-plots-c"/>.
> The transformation function should
> have the form given by any of &pltr0;, &pltr1;, or &pltr2;.
> + Note that unlike &plshades; and similar PLplot functions
> + which have a <literal><parameter>pltr</parameter></literal>
> + argument, plcont requires that a transformation function be
> + provided in the C interface. There is no fallback if
> + <literal><parameter>pltr</parameter></literal> is NULL. Leaving
> + <literal><parameter>pltr</parameter></literal> as a NULL will
> + lead to undefined behavior.
> </para>
> </listitem>
> </varlistentry>
> diff --git a/src/plcont.c b/src/plcont.c
> index 02b574e..346ed1d 100644
> --- a/src/plcont.c
> +++ b/src/plcont.c
> @@ -485,9 +485,17 @@ c_plcont(PLFLT **f, PLINT nx, PLINT ny, PLINT kx, PLINT
> lx,
> PLfGrid2 grid;
>
> grid.f = f;
> - plfcont(plf2eval2, (PLPointer) &grid,
> - nx, ny, kx, lx, ky, ly, clevel, nlevel,
> - pltr, pltr_data);
> + if (pltr == NULL) {
> + // If pltr is undefined, use a simple identity transformation
> + plfcont(plf2eval2, (PLPointer) &grid,
> + nx, ny, kx, lx, ky, ly, clevel, nlevel,
> + pltr0, pltr_data);
> + }
> + else {
> + plfcont(plf2eval2, (PLPointer) &grid,
> + nx, ny, kx, lx, ky, ly, clevel, nlevel,
> + pltr, pltr_data);
> + }
> }
>
>
> /*--------------------------------------------------------------------------*\
> diff --git a/src/plcont.c b/src/plcont.c
> index 02b574e..c217314 100644
> --- a/src/plcont.c
> +++ b/src/plcont.c
> @@ -485,9 +485,15 @@ c_plcont(PLFLT **f, PLINT nx, PLINT ny, PLINT kx, PLINT
> lx,
> PLfGrid2 grid;
>
> grid.f = f;
> - plfcont(plf2eval2, (PLPointer) &grid,
> - nx, ny, kx, lx, ky, ly, clevel, nlevel,
> - pltr, pltr_data);
> + if (pltr == NULL) {
> + // If pltr is undefined, abort with an error.
> + plabort("plcont: The pltr callback must be defined");
> + }
> + else {
> + plfcont(plf2eval2, (PLPointer) &grid,
> + nx, ny, kx, lx, ky, ly, clevel, nlevel,
> + pltr, pltr_data);
> + }
> }
>
>
> /*--------------------------------------------------------------------------*\
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
> _______________________________________________
> Plplot-devel mailing list
> Plplot-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/plplot-devel
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
