[Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE

[Lukas Slebodnik]

On (26/11/14 16:51), Petr Spacek wrote:
>On 26.11.2014 16:47, Lukas Slebodnik wrote:
>> On (12/11/14 16:34), Petr Spacek wrote:
>>> On 25.2.2014 15:05, Lukas Slebodnik wrote:
>>>> On (25/02/14 09:54), Petr Spacek wrote:
>>>>> On 24.2.2014 18:56, Lukas Slebodnik wrote:
>>>>>> On (24/02/14 16:48), Petr Spacek wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> Drop unnecessary #define _BSD_SOURCE.
>>>>>>>
>>>>>>> --
>>>>>>> Petr^2 Spacek
>>>>>>
>>>>>> >From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001
>>>>>>> From: Petr Spacek <pspacek at redhat.com>
>>>>>>> Date: Mon, 24 Feb 2014 16:48:09 +0100
>>>>>>> Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE.
>>>>>>>
>>>>>>> Signed-off-by: Petr Spacek <pspacek at redhat.com>
>>>>>>> ---
>>>>>>> src/krb5_helper.c | 2 --
>>>>>>> 1 file changed, 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/src/krb5_helper.c b/src/krb5_helper.c
>>>>>>> index d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 100644
>>>>>>> --- a/src/krb5_helper.c
>>>>>>> +++ b/src/krb5_helper.c
>>>>>>> @@ -15,8 +15,6 @@
>>>>>>>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>>>>>>  */
>>>>>>>
>>>>>>> -#define _BSD_SOURCE
>>>>>>> -
>>>>>>> #include <isc/util.h>
>>>>>>> #include <string.h>
>>>>>>> #include <stdlib.h>
>>>>>>> --
>>>>>>> 1.8.5.3
>>>>>>>
>>>>>>
>>>>>> Simo is an author (according to git blame)
>>>>>> He defined this macro due to function setenv
>>>>>>
>>>>> >from man setenv:
>>>>>> NAME
>>>>>>        setenv - change or add an environment variable
>>>>>>
>>>>>> SYNOPSIS
>>>>>>        #include <stdlib.h>
>>>>>>
>>>>>>        int setenv(const char *name, const char *value, int overwrite);
>>>>>>
>>>>>>        int unsetenv(const char *name);
>>>>>>
>>>>>>    Feature Test Macro Requirements for glibc (see feature_test_macros(7)):
>>>>>>
>>>>>>        setenv(), unsetenv():
>>>>>>            _BSD_SOURCE || _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600
>>>>>> ----------------------------------------------------------------------------
>>>>>>
>>>>>> Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included
>>>>>> header file <stdlib.h>. I tested only on fedora 20. It can be used
>>>>>> on the other distributions.
>>>>>>
>>>>>> I would rather let this macro as is.
>>>>>
>>>>> Wow, I didn't expect that somebody will spend time on this :-)
>>>>>
>>>>> See build logs from Fedora 21
>>>>> http://koji.fedoraproject.org/koji/getfile?taskID=6565007&name=build.log
>>>>
>>>> You should have noticed this in the 1st mail. Because it is difference between
>>>> removing unnecessary macro and depprecated usage of macro.
>>>>
>>>> /usr/include/features.h:145:3: error: #warning "_BSD_SOURCE and _SVID_SOURCE
>>>> are deprecated, use _DEFAULT_SOURCE" [-Werror=cpp]
>>>>  # warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE"
>>>>
>>>>> Patches with 'the right' solution are welcome. I'm not going to spend
>>>>> more time on this.
>>>
>>> Attached patch should fix the warning in the 'proper' way, I hope. Without
>>> this patch the warning constantly pops up on Fedora 21.
>>>
>>> -- 
>>> Petr^2 Spacek
>> 
>>>From 873334fb1ede302b3a6cbf52ac8bc7e98a4659f9 Mon Sep 17 00:00:00 2001
>>> From: Petr Spacek <pspacek at redhat.com>
>>> Date: Wed, 12 Nov 2014 16:30:56 +0100
>>> Subject: [PATCH] Replace deprecated macro #define _BSD_SOURCE with
>>> _POSIX_C_SOURCE.
>>>
>>> See
>>> https://sourceware.org/glibc/wiki/Release/2.20#Packaging_Changes
>>> ---
>>> src/krb5_helper.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/krb5_helper.c b/src/krb5_helper.c
>>> index 169d384cddb5ab9fc9cce1f5ec773836a4c383bb..85c8df9f15af839786ded50d41313763f6463579 100644
>>> --- a/src/krb5_helper.c
>>> +++ b/src/krb5_helper.c
>>> @@ -15,7 +15,7 @@
>>>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>>  */
>>>
>>> -#define _BSD_SOURCE
>>> +#define _POSIX_C_SOURCE 200112L /* setenv */
>> I'm not sure wheather it is good idea to define special value.
>> It should be autodetected.
>> 
>> One way could be to test available extensions in configure time
>> AC_USE_SYSTEM_EXTENSIONS.
>> 
>> or use _BSD_SOURCE conditioanally.
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 9e33116..95a1440 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -30,7 +30,7 @@ AC_CHECK_HEADERS([stddef.h stdlib.h string.h strings.h])
>>  AC_TYPE_SIZE_T
>>  
>>  # Checks for library functions.
>> -AC_CHECK_FUNCS([memset strcasecmp strncasecmp])
>> +AC_CHECK_FUNCS([memset strcasecmp strncasecmp setenv])
>>  
>>  # Check if build chain supports symbol visibility
>>  AC_MSG_CHECKING([for -fvisibility=hidden compiler flag])
>> diff --git a/src/krb5_helper.c b/src/krb5_helper.c
>> index d178720..8ce11b8 100644
>> --- a/src/krb5_helper.c
>> +++ b/src/krb5_helper.c
>> @@ -15,7 +15,10 @@
>>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>   */
>>  
>> +#include "config.h"
>> +#ifndef HAVE_SETENV
>>  #define _BSD_SOURCE
>> +#endif /* HAVE_SETENV */
>>  
>>  #include <isc/util.h>
>>  #include <string.h>
>
>Hello,
>
>thank you for your patch but I'm going to stick to standard POSIX way.
>
NACK

You don't know which version of posix is defined on machine.
Hardcoded value is as bad.

LS