[SSSD] [PATCH] Use custom implementation of glibc extension

Pavel Březina pbrezina at redhat.com
Tue Jun 3 10:22:16 UTC 2014


On 05/31/2014 04:04 PM, Lukas Slebodnik wrote:
> On (27/05/14 10:23), Jakub Hrozek wrote:
>> On Tue, May 27, 2014 at 10:08:02AM +0200, Sumit Bose wrote:
>>> On Tue, May 27, 2014 at 09:36:29AM +0200, Lukas Slebodnik wrote:
>>>> On (27/05/14 09:27), Pavel Reichl wrote:
>>>>> On Tue, 2014-05-27 at 09:17 +0200, Lukas Slebodnik wrote:
>>>>>> On (27/05/14 09:11), Pavel Reichl wrote:
>>>>>>> On Tue, 2014-05-27 at 00:19 +0200, Lukas Slebodnik wrote:
>>>>>>>> ehlo,
>>>>>>>>
>>>>>>>> strchrnul() and mempcpy() are GNU extensions to libc
>>>>>>>> This patch add configure time test for this functions and provide
>>>>>>>> custom implementation if functions are not available.
>>>>>>>>
>>>>>>>> patch is attached.
>>>>>>>>
>>>>>>>> LS
>>>>>>>> _______________________________________________
>>>>>>>> sssd-devel mailing list
>>>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>>>
>>>>>>> Hello Lukas,
>>>>>>>
>>>>>>> is it really necessary to have two definitions of
>>>>>>> mempcpy()?
>>>>>> Yes,
>>>>>>
>>>>>> libsss_crypt.so is built either with nss or with libcrypto.
>>>>>> Look into Makefile.am for details.
>>>>>>
>>>>>> LS
>>>>>> _______________________________________________
>>>>>> sssd-devel mailing list
>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>
>>>>> Yes, I'm aware of that. I was just thinking if you could move the
>>>>> definition into separate file and thus avoid code duplication...
>>>>>
>>>> No, there will be no benefit.
>>>> There isn't any shared file between two implementation of libsss_crypt.
>>>> static function can be easily inlined and I don't want to have conditional
>>>> definition of function in header file.
>>>
>>> I wonder if using a "library" like gnulib might be useful in the long
>>> run to handle cases like this? I'm not saying we should do it now, but
> Problematic functions are in old stable code and only in one module.
> SSSD has a lot of other dependencies I do not want to add another one.
>
>>> since you are currently looking at glibc, XOPEN and other extension I
>>> wonder what's your opinion here?
> macro XOPEN is not related to these functions. It is different story :-)
>
>>>
>>> bye,
>>> Sumit
>>
>> Yes, I prefer using code that was tested by others as well.
>>
>> Also, unlike the includes pushed yesterday, I would prefer either a test
>> for these functions
> The function strchrnul is used in confdb function parse_section
> and it is indirectly tested in test_utils, simple_access-tests, sysdb-tests,
> sysdb_ssh-tests
>
> The function mempcpy is used in computation of has sha512.
> I added new test for function s3crypt_sha512.
>
> Thess passes with:
>       --with-crypto=nss
>       --with-crypto=libcrypto
>       with custom implementation of mempcpy
>
> The last one can be tested with next diff:
> diff --git a/src/util/crypto/nss/nss_sha512crypt.c b/src/util/crypto/nss/nss_sha512crypt.c
> index 11194e5..54f6cbd 100644
> --- a/src/util/crypto/nss/nss_sha512crypt.c
> +++ b/src/util/crypto/nss/nss_sha512crypt.c
> @@ -29,11 +29,11 @@
>   #include <sechash.h>
>   #include <pk11func.h>
>
> -#ifndef HAVE_MEMPCPY
> +#undef mempcpy
> +#define mempcpy mymempcpy
>   static void * mempcpy(void *dst, const void *src, size_t len) {
>       return (void*) (((char*)memcpy(dst, src, len)) + len);
>   }
> -#endif /* HAVE_MEMPCPY */
>
>   /* Define our magic string to mark salt for SHA512 "encryption" replacement. */
>   const char sha512_salt_prefix[] = "$6$";
>
> LS

Can you also add a memory leak check? See test_encrypt_decrypt.




More information about the sssd-devel mailing list