Re: Feature patch 1 for plperl [PATCH]

I didn't get any significant feedback from the earlier draft so here's
the finished 'feature patch 1' for plperl. (This builds on my earlier
plperl refactoring patch, and the follow-on ppport.h patch.)

Significant changes from the first draft:
- New GUC plperl.on_perl_init='...perl...' for admin use.
- New GUC plperl.on_trusted_init='...perl...' for plperl user use.
- New GUC plperl.on_untrusted_init='...perl...' for plperlu user use.
- END blocks now run at backend exit (fixes bug #5066).
- Stored procedure subs are now given names ($name__$oid).
- More error checking and reporting.
- Warnings no longer have an extra newline in the NOTICE text.
- Various minor optimizations like pre-growing data structures.

Additional changes from the second draft:
- SPI functions aren't available during plperl.on_*_init execution.
- Added utility functions: quote_literal, quote_nullable, quote_ident,
encode_bytea, decode_bytea, looks_like_number,
encode_array_literal, encode_array_constructor.
- Enabled plperl to "use"/"require" safely by redirecting the require
opcode to code that dies if module not already loaded.
- Corresponding changes to the documentation.

Additional changes in this version:
- Added the missing ', arguments' to docs of spi_exec_prepared().
- Added Util.c to list of files for plperl make clean to delete.

I'll add this to the commitfest.

Tim.

On Jan 8, 2010, at 7:01 AM, Tim Bunce wrote:

> I didn't get any significant feedback from the earlier draft so here's
> the finished 'feature patch 1' for plperl. (This builds on my earlier
> plperl refactoring patch, and the follow-on ppport.h patch.)
>
> Significant changes from the first draft:
> - New GUC plperl.on_perl_init='...perl...' for admin use.
> - New GUC plperl.on_trusted_init='...perl...' for plperl user use.
> - New GUC plperl.on_untrusted_init='...perl...' for plperlu user use.
> - END blocks now run at backend exit (fixes bug #5066).
> - Stored procedure subs are now given names ($name__$oid).
> - More error checking and reporting.
> - Warnings no longer have an extra newline in the NOTICE text.
> - Various minor optimizations like pre-growing data structures.
>
> Additional changes from the second draft:
> - SPI functions aren't available during plperl.on_*_init execution.
> - Added utility functions: quote_literal, quote_nullable, quote_ident,
> encode_bytea, decode_bytea, looks_like_number,
> encode_array_literal, encode_array_constructor.
> - Enabled plperl to "use"/"require" safely by redirecting the require
> opcode to code that dies if module not already loaded.
> - Corresponding changes to the documentation.
>
> Additional changes in this version:
> - Added the missing ', arguments' to docs of spi_exec_prepared().
> - Added Util.c to list of files for plperl make clean to delete.
>
> I'll add this to the commitfest.

These changes all sound great to me, Tim, and if I can ever get my PL/Perl install working again, I'd be glad to find some tuits and review it.

Best,

David

On Fri, Jan 8, 2010 at 10:01 AM, Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> wrote:
> I didn't get any significant feedback from the earlier draft so here's
> the finished 'feature patch 1' for plperl.  (This builds on my earlier
> plperl refactoring patch, and the follow-on ppport.h patch.)
>
> Significant changes from the first draft:
> - New GUC plperl.on_perl_init='...perl...' for admin use.ef
> - New GUC plperl.on_trusted_init='...perl...' for plperl user use.
> - New GUC plperl.on_untrusted_init='...perl...' for plperlu user use.

I kind of thought Tom said these were a bad idea, and I think I kind
of agree. We're not going to support multi-line values for GUCs
AFAIK, so this is going to be pretty kludgy. What about making the
value a comma-separated list of module names to use, or something?

> - END blocks now run at backend exit (fixes bug #5066).
> - Stored procedure subs are now given names ($name__$oid).
> - More error checking and reporting.
> - Warnings no longer have an extra newline in the NOTICE text.
> - Various minor optimizations like pre-growing data structures.
>
> Additional changes from the second draft:
> - SPI functions aren't available during plperl.on_*_init execution.
> - Added utility functions: quote_literal, quote_nullable, quote_ident,
>    encode_bytea, decode_bytea, looks_like_number,
>    encode_array_literal, encode_array_constructor.
> - Enabled plperl to "use"/"require" safely by redirecting the require
>    opcode to code that dies if module not already loaded.
> - Corresponding changes to the documentation.
>
> Additional changes in this version:
> - Added the missing ', arguments' to docs of spi_exec_prepared().
> - Added Util.c to list of files for plperl make clean to delete.
>
> I'll add this to the commitfest.

The rest of this all seems pretty nice, though I haven't read the patch yet.

...Robert

On fre, 2010-01-08 at 15:01 +0000, Tim Bunce wrote:
> I didn't get any significant feedback from the earlier draft so here's
> the finished 'feature patch 1' for plperl.

I think it would help if you could split this up into about 6 to 10
single-feature patches.

On Fri, Jan 08, 2010 at 10:36:43PM -0500, Robert Haas wrote:
> On Fri, Jan 8, 2010 at 10:01 AM, Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> wrote:
> > I didn't get any significant feedback from the earlier draft so here's
> > the finished 'feature patch 1' for plperl.  (This builds on my earlier
> > plperl refactoring patch, and the follow-on ppport.h patch.)
> >
> > Significant changes from the first draft:
> > - New GUC plperl.on_perl_init='...perl...' for admin use.
> > - New GUC plperl.on_trusted_init='...perl...' for plperl user use.
> > - New GUC plperl.on_untrusted_init='...perl...' for plperlu user use.
>
> I kind of thought Tom said these were a bad idea, and I think I kind
> of agree.

Tom had some concerns which I believe I've addressed.
I'd be happy to hear from Tom if he has any remaining concerns.

> We're not going to support multi-line values for GUCs
> AFAIK, so this is going to be pretty kludgy.

I'm not sure what you mean by "this". Typical use-cases would be:
plperl.on_perl_init='use MyStuff;'
plperl.on_trusted_init='$some_global=42';

> What about making the value a comma-separated list of module names to
> use, or something?

That would force people who just want to set some global variable
to write a module. That seems overly painful for no significant gain.
The fact that multi-line values for GUCs aren't supported will naturally
enourage anyone wanting to execute many statements to write a module for
them. That sems like a perfectly reasonable balance.

> > [...]
>
> The rest of this all seems pretty nice, though I haven't read the patch yet.

Thanks Robert. I look forward to your feedback if you do get a chance to read it.

Tim.

Peter Eisentraut wrote:
> On fre, 2010-01-08 at 15:01 +0000, Tim Bunce wrote:
>
>> I didn't get any significant feedback from the earlier draft so here's
>> the finished 'feature patch 1' for plperl.
>>
>
> I think it would help if you could split this up into about 6 to 10
> single-feature patches.
>

I think that's a bit excessive. I'd suggest three patches:

* the new utility functions (quote_literal, decode_bytea etc.) These
should be fairly uncontroversial, but account for a large part of
the patch volume.
* the code relating to library load, interpreter initialization and
termination
* the remainder (function naming, better error checking, enabling
use/require if a lib is already loaded, cleanup and optimization)

We could ask Tim to break up the last, but they are all fairly small
items, and I at least wouldn't be bothered by having them combined. And
having to handle 6 to 10 patches all hitting the same body of code
doesn't sound terrible pleasant either.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Peter Eisentraut wrote:
>> I think it would help if you could split this up into about 6 to 10
>> single-feature patches.

> ... having to handle 6 to 10 patches all hitting the same body of code
> doesn't sound terrible pleasant either.

Indeed. That sounds like rather a mess.

regards, tom lane

Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> writes:
> On Fri, Jan 08, 2010 at 10:36:43PM -0500, Robert Haas wrote:
>> I kind of thought Tom said these were a bad idea, and I think I kind
>> of agree.

> Tom had some concerns which I believe I've addressed.

You haven't addressed them, you've simply ignored them. For the record,
I think it's a bad idea to run arbitrary user-defined code in the
postmaster, and I think it's a worse idea to run arbitrary user-defined
code at backend shutdown (the END-blocks bit). I do not care in the
least what applications you think this might enable --- the negative
consequences for overall system stability seem to me to outweigh any
possible arguments on that side. What happens when the supplied code
has errors, takes an unreasonable amount of time to run, does something
unsafe, depends on the backend not being in an error state already, etc
etc?

I do not have a veto over stuff like this, but if I did, it would
not go in.

>> We're not going to support multi-line values for GUCs
>> AFAIK, so this is going to be pretty kludgy.

> I'm not sure what you mean by "this".

What he means by "this" is defining GUCs in a way that would make people
want to use multi-line values for them. However, that doesn't have
anything to do with my worries ...

regards, tom lane

On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> writes:
>> On Fri, Jan 08, 2010 at 10:36:43PM -0500, Robert Haas wrote:
>>> I kind of thought Tom said these were a bad idea, and I think I kind
>>> of agree.
>
>> Tom had some concerns which I believe I've addressed.
>
> You haven't addressed them, you've simply ignored them.

That's not *completely* true.

http://archives.postgresql.org/pgsql-hackers/2009-12/msg00432.php

> For the record,
> I think it's a bad idea to run arbitrary user-defined code in the
> postmaster, and I think it's a worse idea to run arbitrary user-defined
> code at backend shutdown (the END-blocks bit).  I do not care in the
> least what applications you think this might enable --- the negative
> consequences for overall system stability seem to me to outweigh any
> possible arguments on that side.  What happens when the supplied code
> has errors, takes an unreasonable amount of time to run, does something
> unsafe, depends on the backend not being in an error state already, etc
> etc?

Same thing that happens when you put something stupid into
shared_preload_libraries - you destabilize your database cluster and
the world blows up.

> I do not have a veto over stuff like this, but if I did, it would
> not go in.

I'm not as strongly opposed to this as you are, but I definitely think
there will be some people who shoot themselves in the foot with it. I
don't think it's necessarily more dangerous than
shared_preload_libraries from a theoretical standpoint, but the sheer
fact that this is Perl rather than C means more people will try to do
it, and some of them will manage to take out the whole database
cluster, which will not be awesome.

I think this is a real weakness of our one-process-per-connection
model. If it were possible to recycle backends for new connections,
there would be no need even to consider doing things like this. Yeah,
I know you don't want to do that either, just mentioning it...

>>> We're not going to support multi-line values for GUCs
>>> AFAIK, so this is going to be pretty kludgy.
>
>> I'm not sure what you mean by "this".
>
> What he means by "this" is defining GUCs in a way that would make people
> want to use multi-line values for them.  However, that doesn't have
> anything to do with my worries ...

Well, you did mention it previously.

http://archives.postgresql.org/pgsql-hackers/2009-12/msg00384.php

Anyway, I think you've put this pretty well here: the current
definition will make people WANT to use multi-line values for this,
and we don't support that. I think Tim's example is fairly contrived
- setting a global variable here does not seem likely to be useful to
very many users, and the ones who do want it will likely want also
want multi-line values. What IS likely to be useful is preloading a
bunch of perl modules so that backend startup doesn't take an
unreasonably long time. It's nicer to write:

plperl.on_perl_init='strict,warnings,LDAP,HTML::Parser,Archive::Zip'

rather than:

plperl.on_perl_init='use strict;use warnings;use LDAP;use
HTML::Parser;use Archive::Zip;'

I would strongly suggest to Tim that he rip the portions of this patch
that are related to this feature out and submit them separately so
that we can commit the uncontroversial portions first.

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> What happens when the supplied code
>> has errors, takes an unreasonable amount of time to run, does something
>> unsafe, depends on the backend not being in an error state already, etc
>> etc?

> Same thing that happens when you put something stupid into
> shared_preload_libraries - you destabilize your database cluster and
> the world blows up.

shared_preload_libraries is under the sole control of the DBA, though.
What distresses me about this is the exposure to ordinary users.
In particular, that casual little "atexit" addition appears to mean
that *unprivileged* users can break normal functioning of the database,
eg by delaying or even preventing shutdown. That's a security hole of
the first magnitude. Trying to execute SPI code there could make things
even more fun.

I also still don't care for the whole concept of on_init code from a
theoretical standpoint: as I said before, loading of the plperl shared
library should not be a semantically significant event from the user's
viewpoint. If these GUCs were PGC_POSTMASTER it wouldn't be so bad, but
Tim still seems to want them to be settable inside an individual session
before the library is loaded, which makes the loading extremely visible.
As an example, if people were using such functionality then the DBA
couldn't start preloading plperl for performance without breaking
behavior that some of his users might be depending on.

regards, tom lane

Robert Haas wrote:
> Anyway, I think you've put this pretty well here: the current
> definition will make people WANT to use multi-line values for this,
> and we don't support that. I think Tim's example is fairly contrived
> - setting a global variable here does not seem likely to be useful to
> very many users, and the ones who do want it will likely want also
> want multi-line values. What IS likely to be useful is preloading a
> bunch of perl modules so that backend startup doesn't take an
> unreasonably long time. It's nicer to write:
>
> plperl.on_perl_init='strict,warnings,LDAP,HTML::Parser,Archive::Zip'
>
> rather than:
>
> plperl.on_perl_init='use strict;use warnings;use LDAP;use
> HTML::Parser;use Archive::Zip;'
>

I don't know why you would do either of these things. I at least would
load one module which would in turn load others. So I'd expect to see
something like this:

plperl.on_perl_init = 'use lib "/my/app"; use MyApp::Pg;'

I think the suggestion that somehow people will want to put a huge list
of directives straight into postgresql.conf and that this is a reason
not to provide this facility is on the wrong track completely.

> I would strongly suggest to Tim that he rip the portions of this patch
> that are related to this feature out and submit them separately so
> that we can commit the uncontroversial portions first.
>
>
>

See my previous email. I suggested that Tim send three patches: one for
this controversial stuff, one for the new utility functions for plperl,
and one for the remainder. He and I have discussed it and I believe he
is agreeable to that.

cheers

andrew

On Sun, Jan 10, 2010 at 2:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> What happens when the supplied code
>>> has errors, takes an unreasonable amount of time to run, does something
>>> unsafe, depends on the backend not being in an error state already, etc
>>> etc?
>
>> Same thing that happens when you put something stupid into
>> shared_preload_libraries - you destabilize your database cluster and
>> the world blows up.
>
> shared_preload_libraries is under the sole control of the DBA, though.
> What distresses me about this is the exposure to ordinary users.
> In particular, that casual little "atexit" addition appears to mean
> that *unprivileged* users can break normal functioning of the database,
> eg by delaying or even preventing shutdown.  That's a security hole of
> the first magnitude.  Trying to execute SPI code there could make things
> even more fun.

That's really a separate issue from the on_perl_init stuff, but now
that you mention it it does sound like a serious problem. Preventing
SPI from being executed there is probably feasible, but I don't like
the idea that broken code would cause the database to fail to shut
down, and that's probably not fixable (unless maybe we detach shared
memory before executing this code, or something?).

> I also still don't care for the whole concept of on_init code from a
> theoretical standpoint: as I said before, loading of the plperl shared
> library should not be a semantically significant event from the user's
> viewpoint.  If these GUCs were PGC_POSTMASTER it wouldn't be so bad, but
> Tim still seems to want them to be settable inside an individual session
> before the library is loaded, which makes the loading extremely visible.
> As an example, if people were using such functionality then the DBA
> couldn't start preloading plperl for performance without breaking
> behavior that some of his users might be depending on.

Hmm. OK, I agree: that's a problem.

...Robert

Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>
>> On Sun, Jan 10, 2010 at 1:49 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>>> What happens when the supplied code
>>> has errors, takes an unreasonable amount of time to run, does something
>>> unsafe, depends on the backend not being in an error state already, etc
>>> etc?
>>>
>
>
>> Same thing that happens when you put something stupid into
>> shared_preload_libraries - you destabilize your database cluster and
>> the world blows up.
>>
>
> shared_preload_libraries is under the sole control of the DBA, though.
> What distresses me about this is the exposure to ordinary users.
> In particular, that casual little "atexit" addition appears to mean
> that *unprivileged* users can break normal functioning of the database,
> eg by delaying or even preventing shutdown. That's a security hole of
> the first magnitude. Trying to execute SPI code there could make things
> even more fun.
>

I suspect that could be inhibited at least.

> I also still don't care for the whole concept of on_init code from a
> theoretical standpoint: as I said before, loading of the plperl shared
> library should not be a semantically significant event from the user's
> viewpoint. If these GUCs were PGC_POSTMASTER it wouldn't be so bad, but
> Tim still seems to want them to be settable inside an individual session
> before the library is loaded, which makes the loading extremely visible.
> As an example, if people were using such functionality then the DBA
> couldn't start preloading plperl for performance without breaking
> behavior that some of his users might be depending on.
>
>
>

Well, I think the proposed plperl.on_perl_init setting could be
PGC_POSTMASTER, as I previously commented.

The other settings are intended to be used on interpreter start, AIUI.
Maybe we need to explore the use cases more.

If we made plperl.on_perl_init PGC_POSTMASTER and the other two
PGC_SUSET would that alloy your concerns?

cheers

andrew

On Sun, Jan 10, 2010 at 2:58 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> I don't know why you would do either of these things. I at least would load
> one module which would in turn load others. So I'd expect to see something
> like this:
>
>   plperl.on_perl_init = 'use lib "/my/app"; use MyApp::Pg;'
>
> I think the suggestion that somehow people will want to put a huge list of
> directives straight into postgresql.conf and that this is a reason not to
> provide this facility is on the wrong track completely.

Hmm. I have to admit I didn't think about "use lib". That does seem
like a plausible thing to want to do.

>> I would strongly suggest to Tim that he rip the portions of this patch
>> that are related to this feature out and submit them separately so
>> that we can commit the uncontroversial portions first.
>
> See my previous email. I suggested that Tim send three patches: one for this
> controversial stuff, one for the new utility functions for plperl, and one
> for the remainder. He and I have discussed it and I believe he is agreeable
> to that.

OK, well then just +1 for that.

...Robert

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> As an example, if people were using such functionality then the DBA
>> couldn't start preloading plperl for performance without breaking
>> behavior that some of his users might be depending on.

> If we made plperl.on_perl_init PGC_POSTMASTER and the other two
> PGC_SUSET would that alloy your concerns?

No, they have to all be PGC_POSTMASTER to answer that concern. Only
breaking things for superusers isn't really that big an improvement
over breaking them for everybody.

regards, tom lane

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> As an example, if people were using such functionality then the DBA
>>> couldn't start preloading plperl for performance without breaking
>>> behavior that some of his users might be depending on.
>>>
>
>
>> If we made plperl.on_perl_init PGC_POSTMASTER and the other two
>> PGC_SUSET would that alloy your concerns?
>>
>
> No, they have to all be PGC_POSTMASTER to answer that concern. Only
> breaking things for superusers isn't really that big an improvement
> over breaking them for everybody.
>
>
>

Well, I don't know about Tim but I think I could live with that. And
when we get some actual experience with using them we'll have a better
handle on whether or not it gives us any pain.

cheers

andrew

On Jan 10, 2010, at 11:17 AM, Robert Haas wrote:

> It's nicer to write:
>
> plperl.on_perl_init='strict,warnings,LDAP,HTML::Parser,Archive::Zip'
>
> rather than:
>
> plperl.on_perl_init='use strict;use warnings;use LDAP;use
> HTML::Parser;use Archive::Zip;'

Well, no, because sometimes I just want to load something and not have functions exported (into whatever namespaces ends up calling this). So I might have something like:

plplerl.on_perl_init='use HTML::Entities ();'

Other times I might want those functions exported.

FWIW, Bricolage has a feature like this, and you can only put stuff on one line. It's been there since 2002 or so. No one has ever complained about it; I doubt anyone would complain about this, either.

Best,

David

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> No, they have to all be PGC_POSTMASTER to answer that concern. Only
>> breaking things for superusers isn't really that big an improvement
>> over breaking them for everybody.

> Well, I don't know about Tim but I think I could live with that. And
> when we get some actual experience with using them we'll have a better
> handle on whether or not it gives us any pain.

Upon further review, PGC_POSTMASTER isn't going to work because of this
in guc.c:

/*
* Only allow custom PGC_POSTMASTER variables to be created during shared
* library preload; any later than that, we can't ensure that the value
* doesn't change after startup. This is a fatal elog if it happens; just
* erroring out isn't safe because we don't know what the calling loadable
* module might already have hooked into.
*/
if (context == PGC_POSTMASTER &&
!process_shared_preload_libraries_in_progress)
elog(FATAL, "cannot create PGC_POSTMASTER variables after startup");

We certainly don't want to make it such that plperl *has* to be
preloaded, so PGC_POSTMASTER is out. However, I think PGC_SIGHUP
would be enough to address my basic worry, which is that people
shouldn't be depending on the ability to set these things within
an individual session.

regards, tom lane

Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> writes:
> I didn't get any significant feedback from the earlier draft so here's
> the finished 'feature patch 1' for plperl. (This builds on my earlier
> plperl refactoring patch, and the follow-on ppport.h patch.)

Just looking over this patch, I don't think it's nearly robust enough
against initialization failures. The original code wasn't very good
about that either, but that was (more or less) okay because it was
executing predetermined, pretested code that we really don't expect to
fail. I think the standard has to be a *lot* higher if we are going to
execute user-supplied perl code there. You need to make sure that
things are left in a reasonably consistent, safe state if an error
occurs.

Along the same line, there needs to be more effort put into the errors
that can be thrown when one of these failures happen. The current
messages don't follow our style guidelines very well, and aren't exposed
for translation.

regards, tom lane

On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> I would strongly suggest to Tim that he rip the portions of this patch
>>> that are related to this feature out and submit them separately so
>>> that we can commit the uncontroversial portions first.
>>
>> See my previous email. I suggested that Tim send three patches: one for this
>> controversial stuff, one for the new utility functions for plperl, and one
>> for the remainder. He and I have discussed it and I believe he is agreeable
>> to that.
>
> OK, well then just +1 for that.

I believe we have agreement on this course of action, so I'm going to
mark the current patch as Returned with Feedback. Hopefully Tim will
submit separate patches for each of these three areas in the next day
or two before start-of-CommitFest - personally, I think they should
each get their own thread and their own entry in the CommitFest app,
for ease of tracking and reviewing. YMMV, of course.

...Robert

On Tue, Jan 12, 2010 at 07:06:59PM -0500, Robert Haas wrote:
> On Sun, Jan 10, 2010 at 4:35 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >>> I would strongly suggest to Tim that he rip the portions of this patch
> >>> that are related to this feature out and submit them separately so
> >>> that we can commit the uncontroversial portions first.
> >>
> >> See my previous email. I suggested that Tim send three patches: one for this
> >> controversial stuff, one for the new utility functions for plperl, and one
> >> for the remainder. He and I have discussed it and I believe he is agreeable
> >> to that.
> >
> > OK, well then just +1 for that.
>
> I believe we have agreement on this course of action, so I'm going to
> mark the current patch as Returned with Feedback. Hopefully Tim will
> submit separate patches for each of these three areas in the next day
> or two before start-of-CommitFest

That's my plan. Plus, hopefully at least one more for inter-sp calling.

> personally, I think they should
> each get their own thread and their own entry in the CommitFest app,
> for ease of tracking and reviewing. YMMV, of course.

Yes, that was also my intent.

Tim.