On Mon, 2008-09-22 at 19:51 +0100, Daniel Silverstone wrote: > I'd really appreciate any comments anyone might have
Hope you don't mind lots of comments :-) main.c: If the GPX_SLEEP_TIME environment variable is not set then getenv() returns NULL and causes a segv in atoi(). If you have an error talking to the db (i.e. job->error is set on return from db_find_work()) then cstart will be uninitialised leading to bogus timing information being printed. db.c: #define FN(V) - Should not be required, free(NULL) is fine. mysql.c: - May be useful to print mysql_error(handle) when mysql_real_connect() fails. filename.c: - Would be good to validate getenv(base) != NULL or report error. gpx.c: - The old parser used to accept gz, bzip2, tar, zip etc, is this still supported? In gpx_handle_end_element() - It looks like you've made ctx->got_ele mandatory. I'm not sure the old code enforced this. - Looks like you are missing a check of lat/long against +-90/180 limits In gpx_parse_coord() - It would be worth checking to see if anyone has uploaded any files which use ',' as the decimal separator as is used in some locales. I've no idea if this is strictly a legal in a GPX file. image.c: gpx_parse_coord() - May want to emit an error if fopen(outfilename, "wb") fails mercator.c: mercator_sheet_y() - Bad things happen if you try to project 90 degrees of latitude into mercator. Clamping the input range to +-85.0511 degrees would fit the limits of the normal slippy map. interpolate.c: interpolate() - missing fclose(inputfile) ? Jon _______________________________________________ dev mailing list dev@openstreetmap.org http://lists.openstreetmap.org/listinfo/dev