Re: [v9.1] sepgsql - userspace access vector cache

On Fri, Aug 19, 2011 at 10:31 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Aug 19, 2011 at 10:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Why not just:
>>
>>> SHLIB_LINK = -lselinux
>>
>> I wouldn't have any particular objection to that (although I think it's
>> supposed to be += here).
>
> Oh, right.
>
>> I don't see that any of the other changes
>> Kaigai proposed are helpful, though.
>
> I was coming to the same conclusion.  I sort of liked his idea of
> sticking a conditional #error directive in the header files to make it
> more clear why it was failing.  But on closer examination there's
> really no benefit: it gets lost in a sea of other failures, and if you
> have to look through the failure messages anyway you may as well
> notice that the #include of <selinux/selinux.h> failed as anything
> else.  So I think changing that line to link with libselinux
> unconditionally is about as well as we can do.
>
>>> Similarly, in the case of xml2 we have:
>>
>>> SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))
>>
>>> For xslt, it probably makes sense to filter it out if it wasn't found,
>>> because the code has ifdefs for USE_XSLT that do something sensible if
>>> the library is not there.  But I fail to see what the point is of
>>> filtering out xml2, because surely we're doomed if that's not there...
>>> or am I confused?
>>
>> Hmm.  I think it's just that way to make the code look parallel for both
>> libraries.  But I can see potential value in making -lxml2 unconditional
>> --- as you say, that would result in a link failure instead of a
>> silently broken library.
>
> Right.

On further review, if the initial configure was done without
--with-libxml, xml2 is doomed anyway.  There's no value to changing
anything there one way or the other, because it relies not only on
symbols from the library, but also on symbols that are only defined
when PostgreSQL itself is configured with xml support.  In other
words, there's no chance of undetected failure here.

On the other hand, we've worked pretty hard to keep sepgsql at arm's
length from the core server, so as it stands, it's quite easy to build
a busted sepgsql module.

This probably explains why no one's complained about this before, and
I think the appropriate fix is to change just sepgsql.  I would like
to back-patch that (one-line) change into 9.1 as well, to eliminate
this as a foot-gun for anyone who may be packaging that module for the
first time.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company