Re: [SSSD] [PATCH] ELAPI Adding concept of the targets
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/14/2009 04:50 PM, Dmitri Pal wrote:
See the patch comment.
The wiki page will be really helpful during review:
https://fedorahosted.org/sssd/wiki/WikiPage/ELAPIInterface
------------------------------------------------------------------------
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Nack
Please use a shorter struct name than "elapi_target_pass_in_data".
Keeping line lengths to < 80 characters gets difficult.
struct elapi_recv_data or something similar would be sufficient.
Similarly, _ctx is preferred in the SSSD over _context (such as
elapi_target_ctx instead of elapi_target_context)
elapi_internal_target_cleanup_handler() would be better as
elapi_target_free()
Typo in elapi_internal_add_sink: "Sink refernce before call" should be
"Sink reference before call"
In elapi_internal_create_target, you make reference to relaxing the
checks for adding the sinks and being satisfied with at least one sink
in the list. I think a safer approach would be to define sinks with an
attribute of "Mandatory" and "Preferred". If a sink was mandatory and
failed to load, it should be an error condition. If a sink was Preferred
and failed to load, the ELAPI should continue on. If all sinks were
"Preferred" and none loaded, this should also be an error condition.
EALPI_EVT_* macros were probably meant to be ELAPI_EVT_*
When creating a new directory, you also need to git add a new m4
directory (I usually create the empty file m4/.dir to accomplish this)
Your unit tests do not build in a parallel build tree. You need to make
sure that you use the $(srcdir) and $(builddir) variables correctly. Any
path that needs to be relative to the original sources must use
$(srcdir) and any path that needs to be relative to built binaries or
generated scripts needs to use $(builddir)
So, you need to ensure that your elapi_ut_LDADD entries use $(builddir)
to find the libraries to link against, and you need to ensure that the
- -D macros you're passing to the compiler refer to $(srcdir). The
AM_CPPFLAGS are correctly using $(srcdir) as-is.
- --
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkqJsRQACgkQeiVVYja6o6MyKACfSaC+EJ/4kPIULhaXq8Cbovhp
4yAAoIowmsoelojYbT42p0F3vEvCvOSK
=cdLf
-----END PGP SIGNATURE-----