On 07/18/2013 09:20 PM, Peter Eisentraut wrote:
The new jsonfuncs.c has some confusing typedef scheme. For example, it
has a bunch of definitions like this:
typedef struct getState
{
...
} getState, *GetState;
So GetState is a pointer to getState. I have never seen that kind of
convention before.
This then leads to code like
GetState state;
state = palloc0(sizeof(getState));
which has useless mental overhead.
But the fact that GetState is really a pointer isn't hidden at all,
because state is then derefenced with -> or cast from or to void*. So
whatever abstraction might have been intended isn't there at all. And
all of this is an intra-file interface anyway.
And to make this even more confusing, other types such as ColumnIOData
and JsonLexContext are not pointers but structs directly.
I think a more typical PostgreSQL code convention is to use capitalized
camelcase for structs, and use explicit pointers for pointers. I have
attached a patch that cleans this up, in my opinion.
I don't have a problem with this. Sometimes when you've stared at
something for long enough you fail to notice such things, so I welcome
more eyeballs on the code.
Note that this is an externally visible API, so anyone who has written
an extension against it will possibly find it broken. But that's all the
more reason to clean it now, before it makes it to a released version.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers