diff mbox series

Declare reallocarray by default

Message ID 58aa285b-b555-a2be-f207-b2f1d797f43b@redhat.com
State New
Headers show
Series Declare reallocarray by default | expand

Commit Message

Florian Weimer Aug. 28, 2018, 3:15 p.m. UTC
In <https://sourceware.org/ml/libc-alpha/2014-05/msg00506.html>, Joseph 
suggested to use __USE_GNU for this function, but since most of the BSDs 
and Illumos have it, I think __USE_MISC is more appropriate.  We have 
seen some reports of implicit function declarations, resulting in 
pointer truncation.  Declaring the function for _DEFAULT_SOURCE (i.e., 
by default, as long as -ansi isn't specified) would avoid such problems.

Thanks,
Florian

Comments

Carlos O'Donell Aug. 28, 2018, 11:14 p.m. UTC | #1
On 08/28/2018 11:15 AM, Florian Weimer wrote:
> In <https://sourceware.org/ml/libc-alpha/2014-05/msg00506.html>,
> Joseph suggested to use __USE_GNU for this function, but since most
> of the BSDs and Illumos have it, I think __USE_MISC is more
> appropriate.  We have seen some reports of implicit function
> declarations, resulting in pointer truncation.  Declaring the
> function for _DEFAULT_SOURCE (i.e., by default, as long as -ansi
> isn't specified) would avoid such problems.

This seems reasonable to me. The choice of __USE_GNU was perhaps a 
conservative one since it requires _GNU_SOURCE to be defined on the
user side and that enables a lot of extensions. Relaxing reallocarray
to __USE_MISC is a good choice if we are seeing developer problems
with this e.g. reallocarray is detected in some way at runtime, but
with the prototype missing (and compiler warnings ignored), and the
default signature resulting in pointer truncation (int). It's also
a good choice because the BSDs already have it.

Thanks for fixing this.

> Subject: [PATCH] reallocarray: Declare under _DEFAULT_SOURCE
> To: libc-alpha@sourceware.org
> 
> Initially, this function was restricted to _GNU_SOURCE, but experience
> shows that compatibility with existing build systems is improved if we
> declare it under _DEFAULT_SOURCE as well.
> 
> 2018-08-28  Florian Weimer  <fweimer@redhat.com>
> 
> 	* stdlib/stdlib.h (reallocarray): Make available under __USE_MISC.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/NEWS b/NEWS
> index 639fb56c9f..325157c0da 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,9 @@ Major new features:
>  
>  * Optimized generic sinf, cosf, sincosf and tanf.
>  
> +* The reallocarray function is now declared under _DEFAULT_SOURCE, not just
> +  for _GNU_SOURCE, to match BSD environments.
> +

OK.

>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * The glibc.tune tunable namespace has been renamed to glibc.cpu and the
> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 8e23e93557..870e02d904 100644
> --- a/stdlib/stdlib.h
> +++ b/stdlib/stdlib.h
> @@ -549,7 +549,7 @@ extern void *calloc (size_t __nmemb, size_t __size)
>  extern void *realloc (void *__ptr, size_t __size)
>       __THROW __attribute_warn_unused_result__;
>  
> -#ifdef __USE_GNU
> +#ifdef __USE_MISC

OK.

>  /* Re-allocate the previously allocated block in PTR, making the new
>     block large enough for NMEMB elements of SIZE bytes each.  */
>  /* __attribute_malloc__ is not used, because if reallocarray returns
Florian Weimer Aug. 30, 2018, 8:35 a.m. UTC | #2
On 08/29/2018 01:14 AM, Carlos O'Donell wrote:
> On 08/28/2018 11:15 AM, Florian Weimer wrote:
>> In <https://sourceware.org/ml/libc-alpha/2014-05/msg00506.html>,
>> Joseph suggested to use __USE_GNU for this function, but since most
>> of the BSDs and Illumos have it, I think __USE_MISC is more
>> appropriate.  We have seen some reports of implicit function
>> declarations, resulting in pointer truncation.  Declaring the
>> function for _DEFAULT_SOURCE (i.e., by default, as long as -ansi
>> isn't specified) would avoid such problems.
> 
> This seems reasonable to me. The choice of __USE_GNU was perhaps a
> conservative one since it requires _GNU_SOURCE to be defined on the
> user side and that enables a lot of extensions. Relaxing reallocarray
> to __USE_MISC is a good choice if we are seeing developer problems
> with this e.g. reallocarray is detected in some way at runtime, but
> with the prototype missing (and compiler warnings ignored), and the
> default signature resulting in pointer truncation (int). It's also
> a good choice because the BSDs already have it.

Joseph, do you have any comment on this?

Thanks,
Florian
Joseph Myers Aug. 30, 2018, 12:12 p.m. UTC | #3
On Thu, 30 Aug 2018, Florian Weimer wrote:

> > This seems reasonable to me. The choice of __USE_GNU was perhaps a
> > conservative one since it requires _GNU_SOURCE to be defined on the
> > user side and that enables a lot of extensions. Relaxing reallocarray
> > to __USE_MISC is a good choice if we are seeing developer problems
> > with this e.g. reallocarray is detected in some way at runtime, but
> > with the prototype missing (and compiler warnings ignored), and the
> > default signature resulting in pointer truncation (int). It's also
> > a good choice because the BSDs already have it.
> 
> Joseph, do you have any comment on this?

No.  The choice of whether something is included in _DEFAULT_SOURCE is a 
pragmatic one, since it's no longer defined based on a particular set of 
external APIs.  (Of course C++ code still gets _GNU_SOURCE whether it 
wants it or not, until we figure out how to allow libstdc++ to get 
everything its headers need under reserved names.)
diff mbox series

Patch

Subject: [PATCH] reallocarray: Declare under _DEFAULT_SOURCE
To: libc-alpha@sourceware.org

Initially, this function was restricted to _GNU_SOURCE, but experience
shows that compatibility with existing build systems is improved if we
declare it under _DEFAULT_SOURCE as well.

2018-08-28  Florian Weimer  <fweimer@redhat.com>

	* stdlib/stdlib.h (reallocarray): Make available under __USE_MISC.

diff --git a/NEWS b/NEWS
index 639fb56c9f..325157c0da 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@  Major new features:
 
 * Optimized generic sinf, cosf, sincosf and tanf.
 
+* The reallocarray function is now declared under _DEFAULT_SOURCE, not just
+  for _GNU_SOURCE, to match BSD environments.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The glibc.tune tunable namespace has been renamed to glibc.cpu and the
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 8e23e93557..870e02d904 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -549,7 +549,7 @@  extern void *calloc (size_t __nmemb, size_t __size)
 extern void *realloc (void *__ptr, size_t __size)
      __THROW __attribute_warn_unused_result__;
 
-#ifdef __USE_GNU
+#ifdef __USE_MISC
 /* Re-allocate the previously allocated block in PTR, making the new
    block large enough for NMEMB elements of SIZE bytes each.  */
 /* __attribute_malloc__ is not used, because if reallocarray returns