KaiGai Kohei wrote: > I updated patch set of SE-PostgreSQL and related stuff (r1403). > > [1/5] > http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1403.patch
Random observations: heapam.c: you've got a bunch of elog(ERROR) calls in there that should be ereport(ERROR), and should probably have a errcode() on them too. Also the message should be worded like this: "could not insert tuple on \"%s\" due to security reasons" or something like that. I mean: do not use C function names in error messages; quote the table name. tuptoaster.c: same problem heap.c: typo on line 1062, says "el", should say "rel" pgace.h: you have a bunch of "static inline" functions in here. As far as I know this doesn't work in compilers other than GCC :-( See pg_list.h (list_head) for an example. I think we can tolerate this for the three functions in pg_list.h because they are so few and so tiny, but I'm not sure about PGACE because they are a large lot. On the other hand, turning them to real functions would be a performance hit. The pgace worker process ... do your postmaster.c changes work when pgace is disabled? I think it tries to start the worker on every iteration. Maybe it needs more smarts in postmaster.c so that when !sepgsqlIsEnabled() it just doesn't try to start it up. Also, I think there should be a separate function to tell whether a particular PGACE_FEATURE actually needs a worker process; right now the only feature (SELinux) does need it, but is this the case for them all? I didn't delve into many details in the avc, the worker, or anything much here actually -- I just skimmed randomly. This is a really huge patch; sorry I do not have time right now to review it. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
