This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [google] Handle NULL return values in setlocale calls (issue4444046)
- From: Jing Yu <jingyu at google dot com>
- To: Diego Novillo <dnovillo at google dot com>
- Cc: reply at codereview dot appspotmail dot com, libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Sat, 16 Apr 2011 18:17:57 -0700
- Subject: Re: [google] Handle NULL return values in setlocale calls (issue4444046)
- References: <20110416202056.81047AE18B@tobiano.tor.corp.google.com>
Thanks Diego for committing this patch.
Yes, I would like to submit the patch to trunk. The reason is for this
patch is that setlocale() in bionicC always returns NULL (bionicC does
not support setlocale()), however libstdc++ does not handle the NULL
return value of setlocale(xxx,NULL) and thus causes segmentation
fault.
Thanks,
Jing
On Sat, Apr 16, 2011 at 1:20 PM, Diego Novillo <dnovillo@google.com> wrote:
> I'm committing this patch for Jing Yu on google/main.
>
> The patch handles NULL values returned from setlocale. ?Jing, could
> you please describe why this was needed? ?Is this a patch that you
> will want to submit for trunk?
>
> Tested on x86_64. ?Committed to google/main.
>
>
> Diego.
>
> 2011-04-15 ?Jing Yu ?<jingyu@google.com>
>
> ? ? ? ?Google ref 46499.
>
> ? ? ? ?* config/locale/generic/c_locale.cc (__convert_to_v): Handle
> ? ? ? ?NULL return value from setlocale.
> ? ? ? ?* config/locale/generic/c_locale.h (__convert_from_v): Likewise.
> ? ? ? ?* config/locale/generic/time_members.cc (_M_put): Likewise.
>
> diff --git a/libstdc++-v3/config/locale/generic/c_locale.cc b/libstdc++-v3/config/locale/generic/c_locale.cc
> index fb9b425..7e34fcf 100644
> --- a/libstdc++-v3/config/locale/generic/c_locale.cc
> +++ b/libstdc++-v3/config/locale/generic/c_locale.cc
> @@ -52,10 +52,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? ? {
> ? ? ? // Assumes __s formatted for "C" locale.
> ? ? ? char* __old = setlocale(LC_ALL, 0);
> - ? ? ?const size_t __len = strlen(__old) + 1;
> - ? ? ?char* __sav = new char[__len];
> - ? ? ?memcpy(__sav, __old, __len);
> - ? ? ?setlocale(LC_ALL, "C");
> + ? ? ?char* __sav = NULL;
> + ? ? ?if (__old != NULL)
> + ? ? ? ?{
> + ? ? ? ? ?const size_t __len = strlen(__old) + 1;
> + ? ? ? ? ?__sav = new char[__len];
> + ? ? ? ? ?memcpy(__sav, __old, __len);
> + ? ? ? ? ?setlocale(LC_ALL, "C");
> + ? ? ? ?}
> ? ? ? char* __sanity;
> ? ? ? bool __overflow = false;
>
> @@ -117,10 +121,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? ? {
> ? ? ? // Assumes __s formatted for "C" locale.
> ? ? ? char* __old = setlocale(LC_ALL, 0);
> - ? ? ?const size_t __len = strlen(__old) + 1;
> - ? ? ?char* __sav = new char[__len];
> - ? ? ?memcpy(__sav, __old, __len);
> - ? ? ?setlocale(LC_ALL, "C");
> + ? ? ?char* __sav = NULL;
> + ? ? ?if (__old != NULL)
> + ? ? ? ?{
> + ? ? ? ? ?const size_t __len = strlen(__old) + 1;
> + ? ? ? ? ?__sav = new char[__len];
> + ? ? ? ? ?memcpy(__sav, __old, __len);
> + ? ? ? ? ?setlocale(LC_ALL, "C");
> + ? ? ? ?}
> ? ? ? char* __sanity;
>
> ?#if !__DBL_HAS_INFINITY__
> @@ -162,10 +170,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? ? {
> ? ? ? // Assumes __s formatted for "C" locale.
> ? ? ? char* __old = setlocale(LC_ALL, 0);
> - ? ? ?const size_t __len = strlen(__old) + 1;
> - ? ? ?char* __sav = new char[__len];
> - ? ? ?memcpy(__sav, __old, __len);
> - ? ? ?setlocale(LC_ALL, "C");
> + ? ? ?char* __sav = NULL;
> + ? ? ?if (__old != NULL)
> + ? ? ? ?{
> + ? ? ? ? ?const size_t __len = strlen(__old) + 1;
> + ? ? ? ? ?__sav = new char[__len];
> + ? ? ? ? ?memcpy(__sav, __old, __len);
> + ? ? ? ? ?setlocale(LC_ALL, "C");
> + ? ? ? ?}
>
> ?#if !__LDBL_HAS_INFINITY__
> ? ? ? errno = 0;
> diff --git a/libstdc++-v3/config/locale/generic/c_locale.h b/libstdc++-v3/config/locale/generic/c_locale.h
> index 2c76000..6e6f673 100644
> --- a/libstdc++-v3/config/locale/generic/c_locale.h
> +++ b/libstdc++-v3/config/locale/generic/c_locale.h
> @@ -60,7 +60,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? {
> ? ? char* __old = std::setlocale(LC_NUMERIC, 0);
> ? ? char* __sav = 0;
> - ? ?if (__builtin_strcmp(__old, "C"))
> + ? ?if (__old != NULL && __builtin_strcmp(__old, "C"))
> ? ? ? {
> ? ? ? ?const size_t __len = __builtin_strlen(__old) + 1;
> ? ? ? ?__sav = new char[__len];
> diff --git a/libstdc++-v3/config/locale/generic/time_members.cc b/libstdc++-v3/config/locale/generic/time_members.cc
> index 3031075..fb9eb6e 100644
> --- a/libstdc++-v3/config/locale/generic/time_members.cc
> +++ b/libstdc++-v3/config/locale/generic/time_members.cc
> @@ -45,10 +45,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? ? ? ? ? const tm* __tm) const throw()
> ? ? {
> ? ? ? char* __old = setlocale(LC_ALL, 0);
> - ? ? ?const size_t __llen = strlen(__old) + 1;
> - ? ? ?char* __sav = new char[__llen];
> - ? ? ?memcpy(__sav, __old, __llen);
> - ? ? ?setlocale(LC_ALL, _M_name_timepunct);
> + ? ? ?char* __sav = NULL;
> + ? ? ?if (__old != NULL)
> + ? ? ? ?{
> + ? ? ? ? ?const size_t __llen = strlen(__old) + 1;
> + ? ? ? ? ?__sav = new char[__llen];
> + ? ? ? ? ?memcpy(__sav, __old, __llen);
> + ? ? ? ? ?setlocale(LC_ALL, _M_name_timepunct);
> + ? ? ? ?}
> ? ? ? const size_t __len = strftime(__s, __maxlen, __format, __tm);
> ? ? ? setlocale(LC_ALL, __sav);
> ? ? ? delete [] __sav;
> @@ -130,10 +134,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> ? ? ? ? ? const tm* __tm) const throw()
> ? ? {
> ? ? ? char* __old = setlocale(LC_ALL, 0);
> - ? ? ?const size_t __llen = strlen(__old) + 1;
> - ? ? ?char* __sav = new char[__llen];
> - ? ? ?memcpy(__sav, __old, __llen);
> - ? ? ?setlocale(LC_ALL, _M_name_timepunct);
> + ? ? ?char* __sav = NULL;
> + ? ? ?if (__old != NULL)
> + ? ? ? ?{
> + ? ? ? ? ?const size_t __llen = strlen(__old) + 1;
> + ? ? ? ? ?__sav = new char[__llen];
> + ? ? ? ? ?memcpy(__sav, __old, __llen);
> + ? ? ? ? ?setlocale(LC_ALL, _M_name_timepunct);
> + ? ? ? ?}
> ? ? ? const size_t __len = wcsftime(__s, __maxlen, __format, __tm);
> ? ? ? setlocale(LC_ALL, __sav);
> ? ? ? delete [] __sav;
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4444046
>