I pushed your 25, with some additional minor tweaks. I hope I didn't
break anything; please test.
Fabien COELHO wrote:
> >I don't "prefer" memory leaks -- I prefer interfaces that make sense.
>
> C is not designed to return two things, and if it is what is needed it looks
> awkward whatever is done. The static variable trick is dirty, but it is the
> minimal fuss solution, IMO. So we are only trading awkward code against
> awkward code.
That's true.
> I have very little time available, so I'm trying to minimize the effort.
> I've tried "argue my point with committers", but it has proven very
> ineffective. I've switched to "do whatever is asked if it still works", but
> it is not very effective either.
I understand. Sometimes arguing is better, if you can convince the
other person, but sometimes the other person disagrees with you or they
are just not listening. I don't have any useful advice on what to do,
but frequently resigning to do a stupid thing because somebody suggested
it leads to bad decisions.
> >In fact, with ParsedScript I don't think we need to give a name to the
> >anon struct used for builtin scripts.
>
> It is useful that it has a name so that find_builtin can return it.
So it is. I have kept it, but I used the name BuiltinScript rather than
script_t.
> Version v25 results a script which is then passed as an argument, so it
> avoid the dynamic allocation & later free. Maybe it is better. I had to cut
> short the error handling if a file does not exists, though, and it passes a
> struct by value.
Passing structs by value should work fine, and I don't care much about
the case that a file doesn't exist.
> >No need for N_BUILTIN; we can use lengthof(builtin_script) instead.
>
> Indeed. "lengthof" does not seem to be standard... ok, it is a macro in some
> header file. I really wanted to avoid an ugly sizeof divide hack, but as it
> is hidden elsewhere this is fine.
Right.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services