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(a)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(a)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