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
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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello Lukas,
is it really necessary to have two definitions of mempcpy()?
PR
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@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
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@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@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...
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@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@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.
LS
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@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@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 since you are currently looking at glibc, XOPEN and other extension I wonder what's your opinion here?
bye, Sumit
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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@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@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 since you are currently looking at glibc, XOPEN and other extension I wonder what's your opinion here?
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 or even better, an instance of the target system in the CI system Nikolai is building.
With our nightly builds and the usual tests, there is no chance we know this code works properly.
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@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@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
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@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@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.
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@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@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
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@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@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?
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@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@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
On Tue, 2014-05-27 at 09:36 +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@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@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.
Well, would it really be a bad idea to have one source file for whole SSSD which would conditionally define these libc extensions?
static function can be easily inlined and I don't want to have conditional definition of function in header file.
Neither do I.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (27/05/14 10:08), Pavel Reichl wrote:
On Tue, 2014-05-27 at 09:36 +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@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@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.
Well, would it really be a bad idea to have one source file for whole SSSD which would conditionally define these libc extensions?
In this case, functions could not be static.
LS
sssd-devel@lists.fedorahosted.org