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