[postgis-devel] documentation images

[Mark Cave-Ayland]

Kevin Neufeld wrote:

> Hi all,
> 
> I just committed some code that can generate most of the spatial images 
> used in the documentation.
> 
> Before, the image_src directory contained some svg files that were 
> "supposed" to be the source of the png images used in the docs.  But I 
> know of no way to import svg into PostGIS, I can't import them into JUMP 
> to test or regenerate the svg ... they're pretty useless.
> 
> So, I wrote a small program that will parse WKT files in the same 
> directory using the liblwgeom module.  Then, using ImageMagick, 
> automatically generate the png files of interest.  The advantage here, 
> is that all the images have the same style (fill color, line thinkness, 
> drop shadow, etc) and can be changed just by modifying a few variables.  
> Because the files are simple WKT, it's easy to modify the images or 
> generate new ones.
> 
> It's written in C, and my C skills are still on the upward learning 
> curve :)  Can someone who is proficient at C review the work before I 
> start removing images and from SVN and relying on the app for the 
> generation of the documentation? Since the code is currently isolated, I 
> thought it'd be OK to submit the stuff rather have a wack of .patch 
> files for someone to review.

Hi Kevin,

This looks great. The commit seems reasonably well though out and has a 
nice, easy to read style with good comments. Not bad for a first C 
coding effort. Here are a few general things I noticed on review:


- configure.ac doesn't check for ImageMagick's convert command

- comment in doc/html/image_src/Makefile.in still reads "Build the main 
unit test executable"


More specifically for generator.c:

I see this file appears to have some buffer overflow issues, for example:

Line 315: malloc() seems to be to small; you need to malloc the size of 
the filename, plus 10 chars for the "../images/" prefix, plus 1 for the 
trailing zero byte to indicate the length of the string. Don't worry 
about the sizeof(char) multiplier - a char is always 1 byte.

It's generally considered good practice to always use the string "n" 
functions such as snprintf()/strncat() rather than the plain versions 
since if the end of the output buffer is reached the output will simply 
be truncated rather than causing a segfault.

Finally, use of the "rm" command on line 262 is not great for 
portability; check out remove() from the standard C libraries instead.


HTH,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063