[SSSD] [PATCH] UTIL: Function 2string for enum sss_cli_command

Sumit Bose sbose at redhat.com
Wed Jul 8 13:26:52 UTC 2015


On Wed, Jul 08, 2015 at 02:13:42PM +0200, Petr Cech wrote:
> Hi!
> 
> https://fedorahosted.org/sssd/ticket/2703
> 
> It's my first patch to this ticket. It is simple transforming of number of
> command to the string.

Hi Petr,

welcome and thank you for your first patch. Besides Pavel's suggestions
I have some general comments as well.

- There is pamcmd2str() which does a similar job for the backend code
  but I think it is becoming redundant with your patch. Can you remove
  this call and use your's where appropriate?

- I haven't tested it, but I'm pretty sure that the PAM module pam_sss
  which is build from pam_sss.c and some other files is broken in debug
  mode with your patch because sss_log.c is not used when building it and
  hence sss_cli_command_2string() will be undefined. You do not see this
  during compilation or even during 'make check' because the 'D' macro
  is only evaluate if PAM_DEBUG is defined during compilation. If you
  run something like 'make CFLAGS+="-DPAM_DEBUG" check' the dlopen test
  should fail with your patch.

  Since the PAM module pam_sss.so might be loaded by any kind of
  processes at runtime we try to keep it as simple as possible and try
  to add as few dependencies as possible. If you search the Makefile.am
  for pam_sss_la_SOURCES you will see that besides source files from the
  sss_client directory we only add atomic_io.c and authtok-utils.c which
  both contain only a single function with no special dependencies.

  I would suggest that you put sss_cli_command_2string() in a file on
  its own similar like atomic_io.c or authtok-utils.c. And add this file
  to pam_sss_la_SOURCES and libsss_debug_la_SOURCES in Makefile.am. I
  leave it up to you to decide what would be a good place for this file.
  The sss_client directory because the enum sss_cli_command is defined
  here as well or the util directory because the main usage for it is in
  the SSSD code and not in the pam_sss module.

bye,
Sumit



More information about the sssd-devel mailing list