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

Petr Cech pcech at redhat.com
Thu Jul 9 12:03:19 UTC 2015


Hi!

There is my repaired patch. All of yours comments were helpful.

I renamed the function to sss_cmd2str(), but maybe it could be 
sss_cli_cmd2str(). I am not sure with it, but if it is better, I will 
rename it again.

Petr

On 07/08/2015 03:26 PM, Sumit Bose wrote:
> 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
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-UTIL-Function-2string-for-enum-sss_cli_command.patch
Type: text/x-patch
Size: 16738 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150709/888667c5/attachment.bin>


More information about the sssd-devel mailing list