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

Reply via email to