From: Alan W. I. <ir...@be...> - 2010-12-23 00:59:59
|
On 2010-12-22 23:15-0000 Andrew Ross wrote: > Can I put libgdal on the table as one likely candidate for a mapping > library. It provides support for most of the main raster and vector > GIS formats. I've not programmed it directly, but it provides > most of the file read / write support for GRASS which is one of the > most developed free GIS packages. Homepage is http://www.gdal.org/ . That does look like a good candidate. A quick check at the website reveals a huge list of vector and bitmapped map formats that it can read. Do you know anything about the kinds of data that can be obtained from libgdal? Full-color map images would be fairly useless to us (except as a background for a plot). We could do some interesting things with gray-scale image data via plimage; 3D topographic data via plcont or plshades; and outline map data via a changed API for plmap. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: Maurice L. <mj...@br...> - 2010-12-23 05:21:22
|
On Wednesday, December 22, 2010 at 23:15:12 (+0000) Andrew Ross writes: > > Thread moved to plplot-devel as this is becoming a discussion on > new developments / improvements. > > On Wed, Dec 22, 2010 at 02:32:24PM -0800, Alan Irwin wrote: > > On 2010-12-22 11:00+0100 Jos? Luis Garc?a Pallero wrote: > > > > > Hello, > > > I think that PLPlot is a great tool. Thanks to the developers for > > > their hard work. I'm using PLPlot in order to draw worldmaps with > > > coastlines. I'm using plfill, plline and plmap functions and I find > > > they very useful. Nevertheless I have some minor problems (I'm using > > > PLPlot 5.9.7 compiled by me on a Debian GNU/Linux Sid). > > > > > > Using plfill I obtain a the message > > > > > > *** PLPLOT WARNING *** > > > plfill: too many points in polygon > > > > > > using polygons with 1000 vertices. Can I change the maximum number of > > > vertices at runtime or must I recompile the library? > > > [snip] > > Arjen has already answered you in general terms on the PL_MAXPOLY > > question (currently set at a hard limit of 256 in include/plplotP.h). > > If you grep through our code for that symbol, you will notice a number > > of fixed arrays that are dimensioned with that value. I would welcome > > a patch to replace all of those with malloced (and freed) memory so we > > could do away with the PL_MAXPOLY limit completely. A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there are heap-vs-stack performance implications -- e.g. directly moving from a fixed allocation to a malloc/free each time plfill() is called could suck for the many small-n-vertices polygon case. Using per-stream polyline buffers is better but more convoluted. The short term solution is definitely just recompile with a higher limit. -- Maurice LeBrun |
From: Alan W. I. <ir...@be...> - 2010-12-23 07:44:06
|
On 2010-12-22 22:05-0700 Maurice LeBrun wrote: > On Wednesday, December 22, 2010 at 23:15:12 (+0000) Andrew Ross writes: > > > > Thread moved to plplot-devel as this is becoming a discussion on > > new developments / improvements. > > > > On Wed, Dec 22, 2010 at 02:32:24PM -0800, Alan Irwin wrote: > > > On 2010-12-22 11:00+0100 Jos? Luis Garc?a Pallero wrote: > > > > > > > Hello, > > > > I think that PLPlot is a great tool. Thanks to the developers for > > > > their hard work. I'm using PLPlot in order to draw worldmaps with > > > > coastlines. I'm using plfill, plline and plmap functions and I find > > > > they very useful. Nevertheless I have some minor problems (I'm using > > > > PLPlot 5.9.7 compiled by me on a Debian GNU/Linux Sid). > > > > > > > > Using plfill I obtain a the message > > > > > > > > *** PLPLOT WARNING *** > > > > plfill: too many points in polygon > > > > > > > > using polygons with 1000 vertices. Can I change the maximum number of > > > > vertices at runtime or must I recompile the library? > > > > > [snip] > > > > Arjen has already answered you in general terms on the PL_MAXPOLY > > > question (currently set at a hard limit of 256 in include/plplotP.h). > > > If you grep through our code for that symbol, you will notice a number > > > of fixed arrays that are dimensioned with that value. I would welcome > > > a patch to replace all of those with malloced (and freed) memory so we > > > could do away with the PL_MAXPOLY limit completely. > > A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there are > heap-vs-stack performance implications -- e.g. directly moving from a fixed > allocation to a malloc/free each time plfill() is called could suck for the > many small-n-vertices polygon case. Using per-stream polyline buffers is > better but more convoluted. The short term solution is definitely just > recompile with a higher limit. Thanks for reminding us of these good points. Also, PL_MAXPOLY does not constrain the number of points making up a line with plline since the approach appears to be to break up long lines into segments that are smaller than PL_MAXPOLY. So the issue is only how to modify the plfill and plgradient cases so there is no fixed number of points limitation. Which reminds me I am still not done with my alternative plfill and plgradient approaches which do break up the filled area into logical pieces depending on how the polygon clipping path intersects with the edges of the filled area. So perhaps the best thing to do at the moment is to put further discussion and development on hold until I can finish up that work (quite a few months from now the way other things are going). But as part of that work I do plan to remove the number of points limitation. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: Arjen M. <arj...@de...> - 2010-12-23 08:24:21
|
Hi, On 2010-12-23 06:05, Maurice LeBrun wrote: > > A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there are > heap-vs-stack performance implications -- e.g. directly moving from a fixed > allocation to a malloc/free each time plfill() is called could suck for the > many small-n-vertices polygon case. Using per-stream polyline buffers is > better but more convoluted. The short term solution is definitely just > recompile with a higher limit. > You might solve the performance issue by keeping reasonably small fixed arrays around and allocate a set of larger ones if needed. The actual algorithm would point to either these fixed arrays or to the allocated arrays. I suspect that with large sets of points the time required for allocation is dwarfed by the time required for filling the polygons. Regards, Arjen |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 08:55:22
Attachments:
plfill.c.patch
|
> On 2010-12-22 22:05-0700 Maurice LeBrun wrote: >> A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there >> are >> heap-vs-stack performance implications -- e.g. directly moving from a >> fixed >> allocation to a malloc/free each time plfill() is called could suck for >> the >> many small-n-vertices polygon case. Using per-stream polyline buffers is >> better but more convoluted. The short term solution is definitely just >> recompile with a higher limit. Attached I send a patch for svn plfill() that uses malloc/free in case of n > PL_MAXPOLY-1. I've used a behavior similar to the function plP_plfclp() in plfill.c. plP_plfclp tests if the number of point is greater than PL_MAXPOLY and uses static or dynamic array in consequence. I've done the same for c_plfill() and c_plfill3(), so for polygons with small number of vertices no overload is done for the using of malloc/free. This only introduces two if's sentences: for test if malloc must be used and for tests if free must be used. And deletes a if sentence: the check if ( n < PL_MAXPOLY ) in the test for determining if the last point is the same as the initial in tle polyline defining the polygon. Can anyone check the patch? Thanks -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 12:26:27
Attachments:
plgradient.c.patch
|
El día 23 de diciembre de 2010 09:55, José Luis García Pallero <jgp...@gm...> escribió: >> On 2010-12-22 22:05-0700 Maurice LeBrun wrote: >>> A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there >>> are >>> heap-vs-stack performance implications -- e.g. directly moving from a >>> fixed >>> allocation to a malloc/free each time plfill() is called could suck for >>> the >>> many small-n-vertices polygon case. Using per-stream polyline buffers is >>> better but more convoluted. The short term solution is definitely just >>> recompile with a higher limit. > > Attached I send a patch for svn plfill() that uses malloc/free in case > of n > PL_MAXPOLY-1. I've used a behavior similar to the function > plP_plfclp() in plfill.c. plP_plfclp tests if the number of point is > greater than PL_MAXPOLY and uses static or dynamic array in > consequence. I've done the same for c_plfill() and c_plfill3(), so for > polygons with small number of vertices no overload is done for the > using of malloc/free. This only introduces two if's sentences: for > test if malloc must be used and for tests if free must be used. And > deletes a if sentence: the check if ( n < PL_MAXPOLY ) in the test for > determining if the last point is the same as the initial in tle > polyline defining the polygon. > > Can anyone check the patch? > > Thanks Hi again, Attached I send a similar patch (trunk svn plplot) for plgradient.c -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: Arjen M. <arj...@de...> - 2010-12-23 12:36:12
|
Hi José, at a first glance this patch (and the one for plgradient) seems okay. Only one tiny thing: I am not sure free() can be used with a NULL argument. So, I'd say you have to guard against that. (If no one picks this up, I will) Regards, Arjen On 2010-12-23 09:55, José Luis García Pallero wrote: >> On 2010-12-22 22:05-0700 Maurice LeBrun wrote: >>> A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there >>> are >>> heap-vs-stack performance implications -- e.g. directly moving from a >>> fixed >>> allocation to a malloc/free each time plfill() is called could suck for >>> the >>> many small-n-vertices polygon case. Using per-stream polyline buffers is >>> better but more convoluted. The short term solution is definitely just >>> recompile with a higher limit. > > Attached I send a patch for svn plfill() that uses malloc/free in case > of n > PL_MAXPOLY-1. I've used a behavior similar to the function > plP_plfclp() in plfill.c. plP_plfclp tests if the number of point is > greater than PL_MAXPOLY and uses static or dynamic array in > consequence. I've done the same for c_plfill() and c_plfill3(), so for > polygons with small number of vertices no overload is done for the > using of malloc/free. This only introduces two if's sentences: for > test if malloc must be used and for tests if free must be used. And > deletes a if sentence: the check if ( n < PL_MAXPOLY ) in the test for > determining if the last point is the same as the initial in tle > polyline defining the polygon. > > Can anyone check the patch? > > Thanks > > > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > Learn how Oracle Real Application Clusters (RAC) One Node allows customers > to consolidate database storage, standardize their database environment, and, > should the need arise, upgrade to a full multi-node Oracle RAC database > without downtime or disruption > http://p.sf.net/sfu/oracle-sfdevnl > > > ------------------------------------------------------------------------ > > _______________________________________________ > Plplot-devel mailing list > Plp...@li... > https://lists.sourceforge.net/lists/listinfo/plplot-devel DISCLAIMER: This message is intended exclusively for the addressee(s) and may contain confidential and privileged information. If you are not the intended recipient please notify the sender immediately and destroy this message. Unauthorized use, disclosure or copying of this message is strictly prohibited. The foundation 'Stichting Deltares', which has its seat at Delft, The Netherlands, Commercial Registration Number 41146461, is not liable in any way whatsoever for consequences and/or damages resulting from the improper, incomplete and untimely dispatch, receipt and/or content of this e-mail. |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 12:39:34
|
El día 23 de diciembre de 2010 13:35, Arjen Markus <arj...@de...> escribió: > Hi José, > > at a first glance this patch (and the one for plgradient) seems okay. > Only one tiny thing: I am not sure free() can be used with a NULL > argument. So, I'd say you have to guard against that. (If no one > picks this up, I will) Hello, Here: http://linux.die.net/man/3/free says "free(ptr) has already been called before, undefined behaviour occurs. If ptr is NULL, no operation is performed. " I ever check the multiple malloc() returns in this way, following the man page of free() Thanks for your time checking the patches. -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: Arjen M. <arj...@de...> - 2010-12-23 12:41:57
|
Hi José, ah, well, personally I detest this type of magic, it means I have to remember all these little facts, but it means that your patch ought to be applicable as is. I will see to it. Regards, Arjen On 2010-12-23 13:39, José Luis García Pallero wrote: > El día 23 de diciembre de 2010 13:35, Arjen Markus > <arj...@de...> escribió: >> Hi José, >> >> at a first glance this patch (and the one for plgradient) seems okay. >> Only one tiny thing: I am not sure free() can be used with a NULL >> argument. So, I'd say you have to guard against that. (If no one >> picks this up, I will) > > Hello, > Here: http://linux.die.net/man/3/free says > > "free(ptr) has already been called before, undefined behaviour occurs. > If ptr is NULL, no operation is performed. " > > I ever check the multiple malloc() returns in this way, following the > man page of free() > > Thanks for your time checking the patches. > DISCLAIMER: This message is intended exclusively for the addressee(s) and may contain confidential and privileged information. If you are not the intended recipient please notify the sender immediately and destroy this message. Unauthorized use, disclosure or copying of this message is strictly prohibited. The foundation 'Stichting Deltares', which has its seat at Delft, The Netherlands, Commercial Registration Number 41146461, is not liable in any way whatsoever for consequences and/or damages resulting from the improper, incomplete and untimely dispatch, receipt and/or content of this e-mail. |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 12:59:38
|
El día 23 de diciembre de 2010 13:41, Arjen Markus <arj...@de...> escribió: > Hi José, > > ah, well, personally I detest this type of magic, it means I > have to remember all these little facts, but it means that > your patch ought to be applicable as is. I will see to it. > > Regards, > > Arjen > > On 2010-12-23 13:39, José Luis García Pallero wrote: >> >> El día 23 de diciembre de 2010 13:35, Arjen Markus >> <arj...@de...> escribió: >>> >>> Hi José, >>> >>> at a first glance this patch (and the one for plgradient) seems okay. >>> Only one tiny thing: I am not sure free() can be used with a NULL >>> argument. So, I'd say you have to guard against that. (If no one >>> picks this up, I will) >> >> Hello, >> Here: http://linux.die.net/man/3/free says >> >> "free(ptr) has already been called before, undefined behaviour occurs. >> If ptr is NULL, no operation is performed. " >> >> I ever check the multiple malloc() returns in this way, following the >> man page of free() >> >> Thanks for your time checking the patches. >> I see, for example in plfill.c that other allocations looks like: if ( ( xint = (PLINT *) malloc( n * sizeof ( PLINT ) ) ) == NULL ) { plexit( "PlP_pointinpolygon: Insufficient memory" ); } if ( ( yint = (PLINT *) malloc( n * sizeof ( PLINT ) ) ) == NULL ) { plexit( "PlP_pointinpolygon: Insufficient memory" ); } and if ( ( ( xclp = (short *) malloc( ( 2 * npts + 2 ) * sizeof ( short ) ) ) == NULL ) || ( ( yclp = (short *) malloc( ( 2 * npts + 2 ) * sizeof ( short ) ) ) == NULL ) ) { plexit( "plP_plfclp: Insufficient memory" ); } In both examples the plexit() function is used and no previous data is freed (in the first case, if yint malloc fails, xint is not explicitly freed). Maybe I must use plexit() instead of explicitly free. What do you think? I don't know the internals of plexit(), but the documentation says: "This routine is called in case an error is encountered during execution of a PLplot routine. It prints the error message, tries to release allocated resources, calls the handler provided by plsexit and then exits." I don't understand exactly the meaning of "tries to release allocated resources". Means that that it can be fails and produce memory leaks? Cheers. -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: Arjen M. <arj...@de...> - 2010-12-23 13:42:42
|
Hi José, stopping the program in such dramatic circumstances may be the best option. We have had some discussion recently about the best policy, but I am what consensus was reached. I suggest right now we go ahead with your patch and see what corrections are needed later (running out of memory ought to be an unlikely event ...). Regards, Arjen On 2010-12-23 13:59, José Luis García Pallero wrote: > El día 23 de diciembre de 2010 13:41, Arjen Markus > <arj...@de...> escribió: >> Hi José, >> >> ah, well, personally I detest this type of magic, it means I >> have to remember all these little facts, but it means that >> your patch ought to be applicable as is. I will see to it. >> >> Regards, >> >> Arjen >> >> On 2010-12-23 13:39, José Luis García Pallero wrote: >>> El día 23 de diciembre de 2010 13:35, Arjen Markus >>> <arj...@de...> escribió: >>>> Hi José, >>>> >>>> at a first glance this patch (and the one for plgradient) seems okay. >>>> Only one tiny thing: I am not sure free() can be used with a NULL >>>> argument. So, I'd say you have to guard against that. (If no one >>>> picks this up, I will) >>> Hello, >>> Here: http://linux.die.net/man/3/free says >>> >>> "free(ptr) has already been called before, undefined behaviour occurs. >>> If ptr is NULL, no operation is performed. " >>> >>> I ever check the multiple malloc() returns in this way, following the >>> man page of free() >>> >>> Thanks for your time checking the patches. >>> > > I see, for example in plfill.c that other allocations looks like: > > if ( ( xint = (PLINT *) malloc( n * sizeof ( PLINT ) ) ) == NULL ) > { > plexit( "PlP_pointinpolygon: Insufficient memory" ); > } > if ( ( yint = (PLINT *) malloc( n * sizeof ( PLINT ) ) ) == NULL ) > { > plexit( "PlP_pointinpolygon: Insufficient memory" ); > } > > and > > if ( ( ( xclp = (short *) malloc( ( 2 * npts + 2 ) * sizeof ( short ) > ) ) == NULL ) || > ( ( yclp = (short *) malloc( ( 2 * npts + 2 ) * sizeof ( > short ) ) ) == NULL ) ) > { > plexit( "plP_plfclp: Insufficient memory" ); > } > > In both examples the plexit() function is used and no previous data is > freed (in the first case, if yint malloc fails, xint is not explicitly > freed). Maybe I must use plexit() instead of explicitly free. What do > you think? I don't know the internals of plexit(), but the > documentation says: > > "This routine is called in case an error is encountered during > execution of a PLplot routine. It prints the error message, tries to > release allocated resources, calls the handler provided by plsexit > and then exits." > > I don't understand exactly the meaning of "tries to release allocated > resources". Means that that it can be fails and produce memory leaks? > > Cheers. > DISCLAIMER: This message is intended exclusively for the addressee(s) and may contain confidential and privileged information. If you are not the intended recipient please notify the sender immediately and destroy this message. Unauthorized use, disclosure or copying of this message is strictly prohibited. The foundation 'Stichting Deltares', which has its seat at Delft, The Netherlands, Commercial Registration Number 41146461, is not liable in any way whatsoever for consequences and/or damages resulting from the improper, incomplete and untimely dispatch, receipt and/or content of this e-mail. |
From: Alan W. I. <ir...@be...> - 2010-12-23 19:03:08
|
Hi Arjen, José, and Maurice: (I am explicitly addressing Maurice as well since he may know the origin of the free_mem macro that I discuss below.) First, I would like to thank José for his plfill and plgradient patches, and I am glad Arjen is going to apply them. That will make my further changes to plfill and plgradient (many months down the road) easier. Further comments below. On 2010-12-23 14:42+0100 Arjen Markus wrote: > Hi José, > > stopping the program in such dramatic circumstances may be the > best option. We have had some discussion recently about the > best policy, but I am what consensus was reached. I suggest > right now we go ahead with your patch and see what corrections > are needed later (running out of memory ought to be an unlikely > event ...). > > Regards, > José and Arjen have been discussing two separate situations which we should clearly distinguish. 1. What to do right after malloc fails. In this case, I think we should consistently plexit (which calls exit after a error message) as implied by what Arjen said above. malloc failing is an extremely rare error corresponding to out-of-memory when all sorts of bad things are happening in any case. So in my view the arguments expressed on this list before against using plexit (since it not only takes down PLplot but also any interactive environment such as Python or octave that is trying to use it) don't apply that strongly when malloc is failing. 2. How to free the memory after it has been used if the pointer is NULL. This is a non-issue if 1. is dealt with using plexit. However, it is interesting to discuss it anyway. In this case I _think_ José is right and a simple free should do. B&K second edition p 252 says the same as the man page José has been quoting: "does nothing if p is NULL". So since B&K is the C bible, that should be it. Maurice, do you know the origins of the free_mem macro in include/plplotP.h? That does deal with the NULL pointer case for frees, but it also sets the pointer to NULL after every free so I think this macro was meant to be used to guard against double frees of the same memory. OTOH, another interpretation is the author of that macro had run into a badly implemented free implementation that didn't deal properly with a NULL pointer. I would like to get rid of free_mem in the long term if either interpretation is correct. First, I would hope our code is smart enough these days not to attempt double frees. Second, I hope modern implementations of free pay attention to the C standards so they deal properly with the NULL pointer case. In any case, this question should be moot if we uniformly plexit whenever malloc fails. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: Maurice L. <mj...@br...> - 2010-12-23 19:31:37
|
On Thursday, December 23, 2010 at 11:00:26 (-0800) Alan W. Irwin writes: > Maurice, do you know the origins of the free_mem macro in > include/plplotP.h? That does deal with the NULL pointer case for > frees, but it also sets the pointer to NULL after every free so I > think this macro was meant to be used to guard against double frees of > the same memory. OTOH, another interpretation is the author of that > macro had run into a badly implemented free implementation that didn't > deal properly with a NULL pointer. I wrote free_mem as syntactic sugar. I don't think badly implemented free()'s were part of the equation but I can't be sure. So the "sugar" is as follows: - guard against a double free - related: since double free's become a no-op, if control flow becomes thorny you can put a free_mem in anywhere to be sure memory gets freed - the pointer is zeroed out since it bugged me to no end to have bogus pointers around. Especially when debugging. -- Maurice LeBrun |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 21:52:04
Attachments:
plfill.c.patch
plgradient.c.patch
|
Attached I send the (I think) definitive patches for plfill.c and plgradient.c. I've used the plexit() function if any malloc() fails for consistency with the rest of the code. I'll try to find the other sources in wich PL_MAXPOLY appears. Sorry for the previous wrong patches. -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 23:16:15
Attachments:
tkwin.c.patch
xwin.c.patch
|
Hello again, Attached I send patches for trunk/drivers/tkwin.c and trunk/drivers/xwin.c I searched in trunk folder from svn repo ad I found this files containing PL_MAXPOLY: trunk/bindings/tk/plr.c trunk/drivers/tkwin.c trunk/drivers/xfig.c trunk/drivers/xwin.c trunk/src/plarc.c trunk/src/plbuf.c trunk/src/plfill.c trunk/src/plgradient.c trunk/src/plline.c trunk/src/plot3d.c trunk/utils/plrender.c but only plfill, plgradient, tkwin and xwin contains exits if the number of elements is greater than PL_MAXPOLY (I don't konw in deep the code of PLPlot) Cheers -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: Arjen M. <arj...@de...> - 2010-12-24 08:05:34
|
Hi José, I examined these files as well: Some functions would simply return on encountering a large polygon and other (several in plbuf.c for instance) would just happily crash the program. I have implemented a patch for plbuf.c and plot3d.c, but I have not tested them yet, so I want to take some more time. I may get around to do the rest next week, but certainly not this Christmas weekend. I will pick up the patches for xwin.c and tkwin.c, so the remaining files to be patched are: - plr.c - xfig.c - plrender.c (plarc.c does not take arbitrary size polygons, merely uses the macro PL_MAXPOLY, and plline.c splits up a large polyline in pieces). There was a nasty little gotcha in c_plfill3: the vertices are filtered, so that the value of n may be altered. Fixed that in my applying the patch. Regards, Arjen On 2010-12-24 00:16, José Luis García Pallero wrote: > Hello again, > Attached I send patches for trunk/drivers/tkwin.c and trunk/drivers/xwin.c > I searched in trunk folder from svn repo ad I found this files > containing PL_MAXPOLY: > > trunk/bindings/tk/plr.c > trunk/drivers/tkwin.c > trunk/drivers/xfig.c > trunk/drivers/xwin.c > trunk/src/plarc.c > trunk/src/plbuf.c > trunk/src/plfill.c > trunk/src/plgradient.c > trunk/src/plline.c > trunk/src/plot3d.c > trunk/utils/plrender.c > > but only plfill, plgradient, tkwin and xwin contains exits if the > number of elements is greater than PL_MAXPOLY (I don't konw in deep > the code of PLPlot) > > Cheers > DISCLAIMER: This message is intended exclusively for the addressee(s) and may contain confidential and privileged information. If you are not the intended recipient please notify the sender immediately and destroy this message. Unauthorized use, disclosure or copying of this message is strictly prohibited. The foundation 'Stichting Deltares', which has its seat at Delft, The Netherlands, Commercial Registration Number 41146461, is not liable in any way whatsoever for consequences and/or damages resulting from the improper, incomplete and untimely dispatch, receipt and/or content of this e-mail. |
From: José L. G. P. <jgp...@gm...> - 2010-12-23 13:37:49
|
El día 23 de diciembre de 2010 13:35, Arjen Markus <arj...@de...> escribió: > Hi José, > > at a first glance this patch (and the one for plgradient) seems okay. > Only one tiny thing: I am not sure free() can be used with a NULL > argument. So, I'd say you have to guard against that. (If no one > picks this up, I will) > > Regards, > > Arjen And more information about free() in http://www.lysator.liu.se/c/rat/title.html (section 4.10.3.2 http://www.lysator.liu.se/c/rat/d10.html#4-10-3-2) -- ***************************************** José Luis García Pallero jgp...@gm... (o< / / \ V_/_ Use Debian GNU/Linux and enjoy! ***************************************** |
From: David M. <da...@as...> - 2010-12-23 20:32:00
|
On Dec 22, 2010, at 9:05 PM, Maurice LeBrun wrote: > A cautionary note: PL_MAXPOLY impacts a fair amount of code. Also there are > heap-vs-stack performance implications -- e.g. directly moving from a fixed > allocation to a malloc/free each time plfill() is called could suck for the > many small-n-vertices polygon case. Using per-stream polyline buffers is > better but more convoluted. The short term solution is definitely just > recompile with a higher limit. How about alternate versions of these functions that allow the caller to provide (via a pointer) the "workspace buffer"? That would place the onus of malloc/free on the caller, but would also allow the caller to decide how to handle memory allocation failures as well as reuse the same workspace buffer for multiple calls. "We" could even provide a "plpoly_alloc" function that would take "convenient parameters" (e.g. npts) from the caller, compute the buffer size required, allocate a buffer of that size, and return the resulting pointer (or NULL if the allocation failed). The existing implementations of plfill et al. could simply call the new alternate versions passing in pointers to their statically allocated buffers. Just an idea, Dave |