[SSSD] [PATCH] ELAPI Adding concept of the targets

Stephen Gallagher sgallagh at redhat.com
Mon Aug 17 19:35:53 UTC 2009


-----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 at 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-----



More information about the sssd-devel mailing list