Message ID | 20111214170632.GA6579@intel.com |
---|---|
State | New |
Headers | show |
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
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.
> > 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
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.
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.
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 --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