A small rant about coding style for backend functions

I don't want to pick on Teodor in particular, because I've seen other
people do this too, but which of these functions do you find more
readable?

Datum
to_tsquery_byname(PG_FUNCTION_ARGS)
{
PG_RETURN_DATUM(DirectFunctionCall2(
to_tsquery_byid,
ObjectIdGetDatum(name2cfgId((text *) PG_GETARG_POINTER(0), false)),
PG_GETARG_DATUM(1)
));
}

Datum
to_tsquery_byname(PG_FUNCTION_ARGS)
{
text *cfgname = PG_GETARG_TEXT_P(0);
text *txt = PG_GETARG_TEXT_P(1);
Oid cfgId;

cfgId = name2cfgId(cfgname, false);
PG_RETURN_DATUM(DirectFunctionCall2(to_tsquery_byid,
ObjectIdGetDatum(cfgId),
PointerGetDatum(txt)));
}

The main drawback to the V1-call-convention function call mechanism,
compared to ordinary C functions, is that you can't instantly see what
the function arguments are supposed to be. I think that good coding
style demands ameliorating this by declaring and extracting all the
arguments at the top of the function. The above example is bad enough,
but when you have to dig through a function of many lines looking for
GETARG calls in order to know what arguments it expects, it's seriously
annoying and unreadable.

And another thing: use the correct extraction macro for the argument's
type, rather than making something up on the fly. Quite aside from
helping the reader see what the function expects, the first example
above is actually *wrong*, as it will crash on toasted input.

OK, I'm done venting ... back to patch-fixing.

regards, tom lane