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

Lukas Slebodnik lslebodn at redhat.com
Wed Jun 4 10:59:15 UTC 2014


On (04/06/14 11:29), Pavel Březina wrote:
>On 06/03/2014 12:59 PM, Lukas Slebodnik wrote:
>>On (03/06/14 12:22), Pavel Březina wrote:
>>>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.
>>sure
>>memory leak check was added.
>>
>>updated version attached.
>>
>>LS
>
>It is impossible to configure and build with -Werror with these patches. The
>detection of mempcpy then fails.
>
>configure:14593: checking for mempcpy
>configure:14593: gcc -o conftest -m64 -mtune=generic -O0 -ggdb3
>-fstack-protector-all -Wall -Wextra -Wno-sign-compare -Wshadow
>-Wno-unused-parameter -Werror -D_FILE_OFFSET_BITS=64 -D_L
>conftest.c:58:6: error: conflicting types for built-in function 'mempcpy'
>[-Werror]
>cc1: all warnings being treated as errors
>
>
>/home/pbrezina/workspace/sssd/src/util/crypto/nss/nss_sha512crypt.c:33:15:
>error: static declaration of 'mempcpy' follows non-static declaration
>
>Is there a better way of doing that? Or just disabling the -Werror flag for
>the test and enable it afterwards if present in CFLAGS?
>
The configure script should work with any CFLAGS.
I will need to find better way of detection function mempcpy.

LS



More information about the sssd-devel mailing list