diff mbox

PATCH [google/gcc-4_6-branch]: Include <cstddef> in config/locale/generic/c_locale.h

Message ID 20111214170632.GA6579@intel.com
State New
Headers show

Commit Message

H.J. Lu Dec. 14, 2011, 5:06 p.m. UTC
Hi,

Revision 172595:

http://gcc.gnu.org/viewcvs?view=revision&revision=172595

added NULL to config/locale/generic/c_locale.h on google/gcc-4_6-branch.
But <cstddef> isn't included, which lead to build failure since NULL isn't
defined.  This patch includes <cstddef>.  OK for google/gcc-4_6-branch?

H.J.
---
 libstdc++-v3/ChangeLog.hjl                    |    3 +++
 libstdc++-v3/config/locale/generic/c_locale.h |    1 +
 2 files changed, 4 insertions(+), 0 deletions(-)
 create mode 100644 libstdc++-v3/ChangeLog.hjl

Comments

Paolo Carlini Dec. 14, 2011, 6:09 p.m. UTC | #1
Hi,

> Hi,
> 
> Revision 172595:
> 
> http://gcc.gnu.org/viewcvs?view=revision&revision=172595
> 
> added NULL to config/locale/generic/c_locale.h on google/gcc-4_6-branch.
> But <cstddef> isn't included, which lead to build failure since NULL isn't
> defined.  This patch includes <cstddef>.  OK for google/gcc-4_6-branch?

If you are interested in my personal opinion, I have of course nothing to do with this branch, I think this code should not simply use NULL. After all, its C++, right? ;)

Paolo
Diego Novillo Dec. 14, 2011, 6:12 p.m. UTC | #2
On 11-12-14 13:09 , Paolo Carlini wrote:
> Hi,
>
>> Hi,
>>
>> Revision 172595:
>>
>> http://gcc.gnu.org/viewcvs?view=revision&revision=172595
>>
>> added NULL to config/locale/generic/c_locale.h on
>> google/gcc-4_6-branch. But<cstddef>  isn't included, which lead to
>> build failure since NULL isn't defined.  This patch
>> includes<cstddef>.  OK for google/gcc-4_6-branch?
>
> If you are interested in my personal opinion, I have of course
> nothing to do with this branch, I think this code should not simply
> use NULL. After all, its C++, right? ;)

Seems reasonable.  Jing, 172595 was a patch from you.  Would it be the 
same if you just used 0 instead of NULL?


Diego.
Paolo Carlini Dec. 14, 2011, 6:14 p.m. UTC | #3
> 
> Seems reasonable.  Jing, 172595 was a patch from you.  Would it be the same if you just used 0 instead of NULL?

Or nothing.

Paolo
Jing Yu Dec. 14, 2011, 6:43 p.m. UTC | #4
The patch r172595 was intended for bionic setlocale() which always
returns 0. I don't remember I put NULL there initially.

We can simply replace all NULL in r172595 with 0.

I attached the updated patch.
If it is ok, I will commit the patch into google/gcc-4_6 and
google/gcc-4_6-mobile branches.

2011-12-14  H.J. Lu  <hongjiu.lu@intel.com>
                  Jing Yu  <jingyu@google.com>

      * config/locale/generic/c_locale.h (__convert_from_v): Replace
NULL with 0.
      * config/locale/generic/c_locale.cc (__convert_to_v): Likewise
      * config/locale/generic/time_members.cc (_M_put): Likewise

Thanks,
Jing

On Wed, Dec 14, 2011 at 10:12 AM, Diego Novillo <dnovillo@google.com> wrote:
> On 11-12-14 13:09 , Paolo Carlini wrote:
>>
>> Hi,
>>
>>> Hi,
>>>
>>> Revision 172595:
>>>
>>> http://gcc.gnu.org/viewcvs?view=revision&revision=172595
>>>
>>> added NULL to config/locale/generic/c_locale.h on
>>> google/gcc-4_6-branch. But<cstddef>  isn't included, which lead to
>>> build failure since NULL isn't defined.  This patch
>>> includes<cstddef>.  OK for google/gcc-4_6-branch?
>>
>>
>> If you are interested in my personal opinion, I have of course
>> nothing to do with this branch, I think this code should not simply
>> use NULL. After all, its C++, right? ;)
>
>
> Seems reasonable.  Jing, 172595 was a patch from you.  Would it be the same
> if you just used 0 instead of NULL?
>
>
> Diego.
Diego Novillo Dec. 15, 2011, 12:42 p.m. UTC | #5
On 11-12-14 13:43 , Jing Yu wrote:

> Index: config/locale/generic/c_locale.cc
> ===================================================================
> --- config/locale/generic/c_locale.cc	(revision 182019)
> +++ config/locale/generic/c_locale.cc	(working copy)
> @@ -52,8 +52,8 @@
>      {
>        // Assumes __s formatted for "C" locale.
>        char* __old = setlocale(LC_ALL, 0);
> -      char* __sav = NULL;
> -      if (__old != NULL)
> +      char* __sav = 0;
> +      if (__old != 0)


Just 'if (__old)', as Paolo suggested.  Similarly in all the other uses.


OK with that change.


Diego.
Jing Yu Dec. 19, 2011, 10:53 p.m. UTC | #6
Committed to both google/gcc-4_6-google and google/gcc-4_6-mobile
(mobile release branch).

Diego,

I just realize we need this patch for google/gcc-main, since it is a
local patch. OK?

Thanks,
Jing

On Thu, Dec 15, 2011 at 4:42 AM, Diego Novillo <dnovillo@google.com> wrote:
> On 11-12-14 13:43 , Jing Yu wrote:
>
>> Index: config/locale/generic/c_locale.cc
>> ===================================================================
>> --- config/locale/generic/c_locale.cc   (revision 182019)
>> +++ config/locale/generic/c_locale.cc   (working copy)
>> @@ -52,8 +52,8 @@
>>     {
>>       // Assumes __s formatted for "C" locale.
>>       char* __old = setlocale(LC_ALL, 0);
>> -      char* __sav = NULL;
>> -      if (__old != NULL)
>> +      char* __sav = 0;
>> +      if (__old != 0)
>
>
>
> Just 'if (__old)', as Paolo suggested.  Similarly in all the other uses.
>
>
> OK with that change.
>
>
> Diego.
diff mbox

Patch

diff --git a/libstdc++-v3/ChangeLog.hjl b/libstdc++-v3/ChangeLog.hjl
new file mode 100644
index 0000000..b9ca5bd
--- /dev/null
+++ b/libstdc++-v3/ChangeLog.hjl
@@ -0,0 +1,3 @@ 
+2011-12-07  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/locale/generic/c_locale.h: Include <cstddef>.
diff --git a/libstdc++-v3/config/locale/generic/c_locale.h b/libstdc++-v3/config/locale/generic/c_locale.h
index 6e6f673..58c954a 100644
--- a/libstdc++-v3/config/locale/generic/c_locale.h
+++ b/libstdc++-v3/config/locale/generic/c_locale.h
@@ -40,6 +40,7 @@ 
 #pragma GCC system_header
 
 #include <clocale>
+#include <cstddef>
 
 #define _GLIBCXX_NUM_CATEGORIES 0