Re: X.509 Certificate parsing Server (ITS#3184)

[ando@sys-net.it]

m.sahalayev@pgr.salford.ac.uk wrote:

>Full_Name: Mikhail Sahalayev
>Version: 2.2.12
>OS: linux 2.6.4
>URL: ftp://ftp.openldap.org/incoming/M.sahalayev-xps-040512.zip
>Submission from: (NULL) (193.110.17.88)
>
>
>This is an initial release of the X.509 Certificate Parsing Server. Please see
>the supplied documentation for futher details.
>
>  
>

I'm going thru the documentation, and I think this is a very nice piece
of work, surely deserving integration.  I have at least a couple of 
comments,
though, which I know are at least partly shared by other developers.

- the change could be done thru the overlay (or "slapo") mechanism, thus
avoiding any impact on the frontend code.  To reduce the clobbering
of the slapd.conf namespace, it is suggested that all the specific 
parameters
be prefixed with "xps-"; the idea of having a specific .conf file to be
included is reasonable, but it should not be a requirement.
Note that overlays currently apply inside database specific portions
of slapd, but there are plans to introduce "frontend" overlays that apply
before database selection (ITS#3080), which could be of use in case
the XPS needs be applied to any database.
If help with overlays is required, feel free to ask; I may provide you
a template with the fundamental calls you need to fill in.

- it is also suggested that the helper functions specific to XPS be either
delcared as "static" in the xps.c file of the overlay, or prefixed with 
"slap_".

- the overlay should be put in a specific "xps" directory under
"contrib/slapd-modules/"

- the specific  schema could be hard-wired into the overlay code,
much like it's currently done for, e.g. overlays/ppolicy.c or
back-monitor/init.c

- the Write Ahead Log should be intended as a temporary solution, since
OpenLDAP's slapd will soon make available a native transaction mechanism;
as such, I'd prefer to make it very well isolated to allow its replacement
(I havent' looked at the code in detail, so maybe it's OK how it is 
right now).
I note that fflush() is not really a system call, but rather a part of 
the stdio
library made available by the libc.  As such, I'd rather use the sync() 
system call
(note that even sync() does not guarantee data is flushed to the disk; 
it only
schedules the flush before returning!).

- it is suggested that a test be added to the test suite; for directions 
on how
to design a test, please feel free to ask, and use existing tests as 
guidelines.

p.




    SysNet - via Dossi,8 27100 Pavia Tel: +390382573859 Fax: +390382476497