[PATCH] Fixing dive notes escape characters in World map exporter

Dirk Hohndel dirk at hohndel.org
Mon Mar 31 21:50:14 PDT 2014


Stupidly I can't include your patch in my response... oh well.

Quick questions:

a) I don't think we support any architecture where sizeof(char) isn't
just 1 - and we don't do those calculations anywhere else. So I think
you might as well drop them. I forgot to ask about that in the first
patch that I already pushed out

b) I know that different people have different preferences there, but
from an API perspective I would prefer replace_char(string, old, new)

c) I'm not in love with the realloc inside the loop. I realize that the
code is correct and that this maybe isn't the most performance critical
part of our application, but still... you could either brute force it
(worst case assumption of len(string)/len(old)*len(new) or you could
count the occurrences of old and then calculate the correct size of the
allocation

d) error handling. printf(...) and return 0 (and then no handling of the
0 as return value... not cool. You could simply bail and return an empty
string, or return a constant error string "ran out of memory processing
the file". :-)

e) whitespace. Please look at the existing code and the CodingStyle
file. There are quite a few issues in your code regarding spaces after
commas, no spaces inside parenthesis, etc.

Thanks

/D



More information about the subsurface mailing list