* [google] Handle NULL return values in setlocale calls (issue4444046)
@ 2011-04-16 20:30 Diego Novillo
2011-04-17 7:30 ` Jing Yu
0 siblings, 1 reply; 3+ messages in thread
From: Diego Novillo @ 2011-04-16 20:30 UTC (permalink / raw)
To: reply, jingyu, libstdc++, gcc-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [google] Handle NULL return values in setlocale calls (issue4444046)
2011-04-16 20:30 [google] Handle NULL return values in setlocale calls (issue4444046) Diego Novillo
@ 2011-04-17 7:30 ` Jing Yu
2011-04-17 13:11 ` Paolo Carlini
0 siblings, 1 reply; 3+ messages in thread
From: Jing Yu @ 2011-04-17 7:30 UTC (permalink / raw)
To: Diego Novillo; +Cc: reply, libstdc++, gcc-patches
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [google] Handle NULL return values in setlocale calls (issue4444046)
2011-04-17 7:30 ` Jing Yu
@ 2011-04-17 13:11 ` Paolo Carlini
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Carlini @ 2011-04-17 13:11 UTC (permalink / raw)
To: Jing Yu; +Cc: Diego Novillo, reply, libstdc++, gcc-patches
Hi,
> 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.
The patch makes sense and I'm ok with it if you add a short comment
explaining the issue, which we never encountered before. Of course,
longer term we (you ;) may want to add to the library a 4th locale model
beyond the existing ones, some sort of dummy locale model meant for
targets like bionicC, which essentially don't provide locales at all, if
I understand correctly: without a working setlocale how can you switch
from one named locale to another in portable C code?
Paolo.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-17 12:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-16 20:30 [google] Handle NULL return values in setlocale calls (issue4444046) Diego Novillo
2011-04-17 7:30 ` Jing Yu
2011-04-17 13:11 ` Paolo Carlini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).