On Mon, Aug 15, 2016 at 10:20 PM, E.Pasma <pasm...@concepts.nl> wrote:
> 11 aug 2016, Dominique Devienne: > >> From https://www.sqlite.org/c3ref/value_blob.html: >> Please pay particular attention to the fact that the pointer returned from >> [...] sqlite3_value_text(), >> [...] can be invalidated by a subsequent call to [...] >> sqlite3_value_text(), [...] >> > Richard, this extension makes 3x calls to sqlite3_value_text() w/o taking copies of the values, in its "step" aggregate function, yet the doc seems to indicate this is unsafe. Does the doc warning applies *across* "step" invocations, and thus not taking copies is OK here, or it also applies *inside* a single invocation, in which case the first two calls *must* take copies of the string to be safe? > I'd also check sqlite3_value_type() explicitly for SQLITE_TEXT to avoid >> implicit conversions. >> >> Who frees p->result in _final()? No one IMHO. So leak I think. >> > Update: the memory issue is resolved in yodays commit > Yes, Anthony mentioned it to me in a private email. Additional little comments. 1) Use const where appropriate: http://programmers. stackexchange.com/questions/204500/when-and-for-what- purposes-should-the-const-keyword-be-used-in-c-for-variables 2) Turn recursion into iteration (try replacing a long string with 1000 instances of your key. That's a thousand stack frames...) Recursion doesn't scale like iteration (and relying on tail-recursion optimization is unreliable IMHO) 3) Don't reallocate systematically. In 1000x case above, that's a lot of them. 4) Don't strlen(), which is O(N), several time the same string, use a (const) var. 5) sqlite_realloc instead of systematically sqlite_malloc. The allocator often allocs more than needed, and can sometimes avoid moving the string around using strcpy. 6) There's no need to realloc when the value is smaller than the key. Can be done in place. 7) Consider allocating more than necessary, and keeping track of the "capacity" of your buffer. Kinda like std::string's .size() vs .capacity(). 8) Consider a 2-pass approach to limit allocations to just 1 per key. 9) Write a unit test! :) _______________________________________________ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
