"David E. Wheeler" <[EMAIL PROTECTED]> writes: > On Jul 2, 2008, at 09:13, Zdenek Kotala wrote: >> Please, do not type space before asterix: >> char * lcstr, * rcstr -> char *lcstr, *rcstr >> >> and do not put extra space in a brackets >> citextcmp( left, right ) -> citextcmp(left, right)
> Okay. Note that this sort of stuff will mostly be fixed by pg_indent, whether or not David does anything about it. But certainly conforming to the project style to begin with will cause less pain to reviewers' eyeballs. > Um, are you referring to this (at the top): > +// PostgreSQL 8.2 Magic. > +#ifdef PG_MODULE_MAGIC > +PG_MODULE_MAGIC; > +#endif Here however is an outright bug: we do not allow // comments, because we still support ANSI-spec compilers that don't recognize them. > Yeah, I'm a bit confused by this. I followed what's in varlena.c on > this. The comparison functions are documented supposed to return 1, 0, > or -1, which of course would be covered by int. But varstr_cmp(), > which ultimately returns the value, returns all kinds of numbers. So, > perhaps someone could say what it's *supposed* to be? btree cmp functions are allowed to return int32 negative, zero, or positive. There is *not* any requirement that negative or positive results be exactly -1 or +1. However, if you are comparing values that are int32 or wider then you can't just return their difference because it might overflow. >> 3) citext_larger, citext_smaller function have memory leak. You >> don't call PG_FREE_IF_COPY on unused text. > These I also duplicated from varlena.c, and I asked about a potential > memory leak in a previous email. If there's a leak here, would there > not also be a leak in varlena.c? The "leak" is irrelevant for larger/smaller. The only place where it's actually useful to do PG_FREE_IF_COPY is in a btree or hash index support function. In other cases you can assume that you're being called in a memory context that's too short-lived for it to matter. >> 5) There are several commented out lines in CREATE OPERATOR >> statement mostly related to NEGATOR. Is there some reason for that? > I copied it from the original citext.sql. Not sure what effect it has. http://developer.postgresql.org/pgdocs/postgres/xoper-optimization.html regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers