[prev in list] [next in list] [prev in thread] [next in thread] 

List:       haiku-commits
Subject:    [haiku-commits] Change in haiku[master]: libs/posix: Always create backend for locale_t objects
From:       Gerrit <review () review ! haiku-os ! org>
Date:       2022-07-26 17:57:45
Message-ID: 9e0792f5cf98af0b01687613f5634ce105889eb1-HTML () review ! haiku-os ! org
[Download RAW message or body]

From Trung Nguyen <trungnt282910@gmail.com>:

Trung Nguyen has uploaded this change for review. ( https://review.haiku-os.org/c/haiku/+/5505 )


Change subject: libs/posix: Always create backend for locale_t objects
......................................................................

libs/posix: Always create backend for locale_t objects

This fixes the NULL pointer bug when calling uselocale() for
locale_t objects using the C/POSIX locale.

This also simplifies the implementation of several other functions.

Change-Id: I0f62ebde5890364c64e6694ec58d38de43ec6841
---
M src/system/libroot/posix/locale/locale_t.cpp
1 file changed, 29 insertions(+), 55 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/05/5505/1

diff --git a/src/system/libroot/posix/locale/locale_t.cpp b/src/system/libroot/posix/locale/locale_t.cpp
index 70ba38d..515a282 100644
--- a/src/system/libroot/posix/locale/locale_t.cpp
+++ b/src/system/libroot/posix/locale/locale_t.cpp
@@ -38,21 +38,6 @@
     LocaleBackend* backend = (l == LC_GLOBAL_LOCALE) ?
         gGlobalLocaleBackend : (LocaleBackend*)locObj->backend;

-    if (backend == NULL) {
-        newObj->backend = NULL;
-        return (locale_t)newObj;
-    }
-
-    // Check if everything is set to "C" or "POSIX",
-    // and avoid making a backend.
-    const char* localeDescription = backend->SetLocale(LC_ALL, NULL);
-
-    if ((strcasecmp(localeDescription, "POSIX") == 0)
-        || (strcasecmp(localeDescription, "C") == 0)) {
-        newObj->backend = NULL;
-        return (locale_t)newObj;
-    }
-
     BPrivate::ErrnoMaintainer errnoMaintainer;

     LocaleBackend*& newBackend = newObj->backend;
@@ -90,12 +75,9 @@
 freelocale(locale_t l)
 {
     LocaleBackendData* locobj = (LocaleBackendData*)l;
-
-    if (locobj->backend) {
-        LocaleBackend::DestroyBackend(locobj->backend);
-        LocaleDataBridge* databridge = locobj->databridge;
-        delete databridge;
-    }
+	LocaleBackend::DestroyBackend(locobj->backend);
+	LocaleDataBridge* databridge = locobj->databridge;
+	delete databridge;
     delete locobj;
 }

@@ -153,48 +135,40 @@
 	}

 	if (backend == NULL) {
-		// for any locale other than POSIX/C, we try to activate the ICU
-		// backend
-		bool needBackend = false;
-		for (int lc = 0; lc <= LC_LAST; lc++) {
-			if (locales[lc] != NULL && strcasecmp(locales[lc], "POSIX") != 0
-					&& strcasecmp(locales[lc], "C") != 0) {
-				needBackend = true;
-				break;
+		// Earlier implementations check for the C locale and avoid creating the backend.
+		// However, this may not work, as the C locale is not always available in the
+		// global locale when uselocale() is called.
+		// Furthermore, we cannot fail when uselocale() is called for valid locale_t objects
+		// according to the POSIX spec.
+		// Therefore, we always create the backend.
+		backend = LocaleBackend::CreateBackend();
+		if (backend == NULL) {
+			errno = ENOMEM;
+			if (newObject) {
+				delete localeObject;
 			}
+			return (locale_t)0;
 		}
-		if (needBackend) {
-			backend = LocaleBackend::CreateBackend();
-			if (backend == NULL) {
-				errno = ENOMEM;
-				if (newObject) {
-					delete localeObject;
-				}
-				return (locale_t)0;
+		databridge = new (std::nothrow) LocaleDataBridge(false);
+		if (databridge == NULL) {
+			errno = ENOMEM;
+			delete backend;
+			if (newObject) {
+				delete localeObject;
 			}
-			databridge = new (std::nothrow) LocaleDataBridge(false);
-			if (databridge == NULL) {
-				errno = ENOMEM;
-				delete backend;
-				if (newObject) {
-					delete localeObject;
-				}
-				return (locale_t)0;
-			}
-			backend->Initialize(databridge);
+			return (locale_t)0;
 		}
+		backend->Initialize(databridge);
 	}

 	BPrivate::ErrnoMaintainer errnoMaintainer;

-	if (backend != NULL) {
-		for (int lc = 0; lc <= LC_LAST; lc++) {
-			if (locales[lc] != NULL) {
-				locale = backend->SetLocale(lc, locales[lc]);
-				if (lc == LC_ALL) {
-					// skip the rest, LC_ALL overrides
-					break;
-				}
+	for (int lc = 0; lc <= LC_LAST; lc++) {
+		if (locales[lc] != NULL) {
+			locale = backend->SetLocale(lc, locales[lc]);
+			if (lc == LC_ALL) {
+				// skip the rest, LC_ALL overrides
+				break;
 			}
 		}
 	}

--
To view, visit https://review.haiku-os.org/c/haiku/+/5505
To unsubscribe, or for help writing mail filters, visit https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I0f62ebde5890364c64e6694ec58d38de43ec6841
Gerrit-Change-Number: 5505
Gerrit-PatchSet: 1
Gerrit-Owner: Trung Nguyen <trungnt282910@gmail.com>
Gerrit-MessageType: newchange

[Attachment #3 (text/html)]

<p>Trung Nguyen has uploaded this change for <strong>review</strong>.</p><p><a \
href="https://review.haiku-os.org/c/haiku/+/5505">View Change</a></p><pre \
style="font-family: monospace,monospace; white-space: pre-wrap;">libs/posix: Always \
create backend for locale_t objects<br><br>This fixes the NULL pointer bug when \
calling uselocale() for<br>locale_t objects using the C/POSIX locale.<br><br>This \
also simplifies the implementation of several other functions.<br><br>Change-Id: \
I0f62ebde5890364c64e6694ec58d38de43ec6841<br>---<br>M \
src/system/libroot/posix/locale/locale_t.cpp<br>1 file changed, 29 insertions(+), 55 \
deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: \
pre-wrap;">git pull ssh://git.haiku-os.org:22/haiku refs/changes/05/5505/1</pre><pre \
style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git \
a/src/system/libroot/posix/locale/locale_t.cpp \
b/src/system/libroot/posix/locale/locale_t.cpp</span><br><span>index 70ba38d..515a282 \
100644</span><br><span>--- \
a/src/system/libroot/posix/locale/locale_t.cpp</span><br><span>+++ \
b/src/system/libroot/posix/locale/locale_t.cpp</span><br><span>@@ -38,21 +38,6 \
@@</span><br><span>     LocaleBackend* backend = (l == LC_GLOBAL_LOCALE) \
?</span><br><span>         gGlobalLocaleBackend : \
(LocaleBackend*)locObj-&gt;backend;</span><br><span> </span><br><span style="color: \
hsl(0, 100%, 40%);">-    if (backend == NULL) {</span><br><span style="color: hsl(0, \
100%, 40%);">-        newObj-&gt;backend = NULL;</span><br><span style="color: hsl(0, \
100%, 40%);">-        return (locale_t)newObj;</span><br><span style="color: hsl(0, \
100%, 40%);">-    }</span><br><span style="color: hsl(0, 100%, \
40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-    // Check if \
everything is set to &quot;C&quot; or &quot;POSIX&quot;,</span><br><span \
style="color: hsl(0, 100%, 40%);">-    // and avoid making a backend.</span><br><span \
style="color: hsl(0, 100%, 40%);">-    const char* localeDescription = \
backend-&gt;SetLocale(LC_ALL, NULL);</span><br><span style="color: hsl(0, 100%, \
40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-    if \
((strcasecmp(localeDescription, &quot;POSIX&quot;) == 0)</span><br><span \
style="color: hsl(0, 100%, 40%);">-        || (strcasecmp(localeDescription, \
&quot;C&quot;) == 0)) {</span><br><span style="color: hsl(0, 100%, 40%);">-        \
newObj-&gt;backend = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-        \
return (locale_t)newObj;</span><br><span style="color: hsl(0, 100%, 40%);">-    \
}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>     \
BPrivate::ErrnoMaintainer errnoMaintainer;</span><br><span> </span><br><span>     \
LocaleBackend*&amp; newBackend = newObj-&gt;backend;</span><br><span>@@ -90,12 +75,9 \
@@</span><br><span> freelocale(locale_t l)</span><br><span> {</span><br><span>     \
LocaleBackendData* locobj = (LocaleBackendData*)l;</span><br><span style="color: \
hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-    if \
(locobj-&gt;backend) {</span><br><span style="color: hsl(0, 100%, 40%);">-        \
LocaleBackend::DestroyBackend(locobj-&gt;backend);</span><br><span style="color: \
hsl(0, 100%, 40%);">-        LocaleDataBridge* databridge = \
locobj-&gt;databridge;</span><br><span style="color: hsl(0, 100%, 40%);">-        \
delete databridge;</span><br><span style="color: hsl(0, 100%, 40%);">-    \
}</span><br><span style="color: hsl(120, 100%, \
40%);">+	LocaleBackend::DestroyBackend(locobj-&gt;backend);</span><br><span \
style="color: hsl(120, 100%, 40%);">+	LocaleDataBridge* databridge = \
locobj-&gt;databridge;</span><br><span style="color: hsl(120, 100%, 40%);">+	delete \
databridge;</span><br><span>     delete locobj;</span><br><span> }</span><br><span> \
</span><br><span>@@ -153,48 +135,40 @@</span><br><span> 	}</span><br><span> \
</span><br><span> 	if (backend == NULL) {</span><br><span style="color: hsl(0, 100%, \
40%);">-		// for any locale other than POSIX/C, we try to activate the \
ICU</span><br><span style="color: hsl(0, 100%, 40%);">-		// backend</span><br><span \
style="color: hsl(0, 100%, 40%);">-		bool needBackend = false;</span><br><span \
style="color: hsl(0, 100%, 40%);">-		for (int lc = 0; lc &lt;= LC_LAST; lc++) \
{</span><br><span style="color: hsl(0, 100%, 40%);">-			if (locales[lc] != NULL \
&amp;&amp; strcasecmp(locales[lc], &quot;POSIX&quot;) != 0</span><br><span \
style="color: hsl(0, 100%, 40%);">-					&amp;&amp; strcasecmp(locales[lc], \
&quot;C&quot;) != 0) {</span><br><span style="color: hsl(0, 100%, \
40%);">-				needBackend = true;</span><br><span style="color: hsl(0, 100%, \
40%);">-				break;</span><br><span style="color: hsl(120, 100%, 40%);">+		// Earlier \
implementations check for the C locale and avoid creating the \
backend.</span><br><span style="color: hsl(120, 100%, 40%);">+		// However, this may \
not work, as the C locale is not always available in the</span><br><span \
style="color: hsl(120, 100%, 40%);">+		// global locale when uselocale() is \
called.</span><br><span style="color: hsl(120, 100%, 40%);">+		// Furthermore, we \
cannot fail when uselocale() is called for valid locale_t objects</span><br><span \
style="color: hsl(120, 100%, 40%);">+		// according to the POSIX \
spec.</span><br><span style="color: hsl(120, 100%, 40%);">+		// Therefore, we always \
create the backend.</span><br><span style="color: hsl(120, 100%, 40%);">+		backend = \
LocaleBackend::CreateBackend();</span><br><span style="color: hsl(120, 100%, \
40%);">+		if (backend == NULL) {</span><br><span style="color: hsl(120, 100%, \
40%);">+			errno = ENOMEM;</span><br><span style="color: hsl(120, 100%, 40%);">+			if \
(newObject) {</span><br><span style="color: hsl(120, 100%, 40%);">+				delete \
localeObject;</span><br><span> 			}</span><br><span style="color: hsl(120, 100%, \
40%);">+			return (locale_t)0;</span><br><span> 		}</span><br><span style="color: \
hsl(0, 100%, 40%);">-		if (needBackend) {</span><br><span style="color: hsl(0, 100%, \
40%);">-			backend = LocaleBackend::CreateBackend();</span><br><span style="color: \
hsl(0, 100%, 40%);">-			if (backend == NULL) {</span><br><span style="color: hsl(0, \
100%, 40%);">-				errno = ENOMEM;</span><br><span style="color: hsl(0, 100%, \
40%);">-				if (newObject) {</span><br><span style="color: hsl(0, 100%, \
40%);">-					delete localeObject;</span><br><span style="color: hsl(0, 100%, \
40%);">-				}</span><br><span style="color: hsl(0, 100%, 40%);">-				return \
(locale_t)0;</span><br><span style="color: hsl(120, 100%, 40%);">+		databridge = new \
(std::nothrow) LocaleDataBridge(false);</span><br><span style="color: hsl(120, 100%, \
40%);">+		if (databridge == NULL) {</span><br><span style="color: hsl(120, 100%, \
40%);">+			errno = ENOMEM;</span><br><span style="color: hsl(120, 100%, \
40%);">+			delete backend;</span><br><span style="color: hsl(120, 100%, 40%);">+			if \
(newObject) {</span><br><span style="color: hsl(120, 100%, 40%);">+				delete \
localeObject;</span><br><span> 			}</span><br><span style="color: hsl(0, 100%, \
40%);">-			databridge = new (std::nothrow) LocaleDataBridge(false);</span><br><span \
style="color: hsl(0, 100%, 40%);">-			if (databridge == NULL) {</span><br><span \
style="color: hsl(0, 100%, 40%);">-				errno = ENOMEM;</span><br><span style="color: \
hsl(0, 100%, 40%);">-				delete backend;</span><br><span style="color: hsl(0, 100%, \
40%);">-				if (newObject) {</span><br><span style="color: hsl(0, 100%, \
40%);">-					delete localeObject;</span><br><span style="color: hsl(0, 100%, \
40%);">-				}</span><br><span style="color: hsl(0, 100%, 40%);">-				return \
(locale_t)0;</span><br><span style="color: hsl(0, 100%, 40%);">-			}</span><br><span \
style="color: hsl(0, 100%, \
40%);">-			backend-&gt;Initialize(databridge);</span><br><span style="color: hsl(120, \
100%, 40%);">+			return (locale_t)0;</span><br><span> 		}</span><br><span \
style="color: hsl(120, 100%, \
40%);">+		backend-&gt;Initialize(databridge);</span><br><span> 	}</span><br><span> \
</span><br><span> 	BPrivate::ErrnoMaintainer errnoMaintainer;</span><br><span> \
</span><br><span style="color: hsl(0, 100%, 40%);">-	if (backend != NULL) \
{</span><br><span style="color: hsl(0, 100%, 40%);">-		for (int lc = 0; lc &lt;= \
LC_LAST; lc++) {</span><br><span style="color: hsl(0, 100%, 40%);">-			if \
(locales[lc] != NULL) {</span><br><span style="color: hsl(0, 100%, 40%);">-				locale \
= backend-&gt;SetLocale(lc, locales[lc]);</span><br><span style="color: hsl(0, 100%, \
40%);">-				if (lc == LC_ALL) {</span><br><span style="color: hsl(0, 100%, \
40%);">-					// skip the rest, LC_ALL overrides</span><br><span style="color: hsl(0, \
100%, 40%);">-					break;</span><br><span style="color: hsl(0, 100%, \
40%);">-				}</span><br><span style="color: hsl(120, 100%, 40%);">+	for (int lc = 0; \
lc &lt;= LC_LAST; lc++) {</span><br><span style="color: hsl(120, 100%, 40%);">+		if \
(locales[lc] != NULL) {</span><br><span style="color: hsl(120, 100%, \
40%);">+			locale = backend-&gt;SetLocale(lc, locales[lc]);</span><br><span \
style="color: hsl(120, 100%, 40%);">+			if (lc == LC_ALL) {</span><br><span \
style="color: hsl(120, 100%, 40%);">+				// skip the rest, LC_ALL \
overrides</span><br><span style="color: hsl(120, 100%, \
40%);">+				break;</span><br><span> 			}</span><br><span> 		}</span><br><span> \
}</span><br><span></span><br></pre><p>To view, visit <a \
href="https://review.haiku-os.org/c/haiku/+/5505">change 5505</a>. To unsubscribe, or \
for help writing mail filters, visit <a \
href="https://review.haiku-os.org/settings">settings</a>.</p><div itemscope \
itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" \
itemtype="http://schema.org/ViewAction"><link itemprop="url" \
href="https://review.haiku-os.org/c/haiku/+/5505"/><meta itemprop="name" \
content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: haiku </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: \
I0f62ebde5890364c64e6694ec58d38de43ec6841 </div> <div style="display:none"> \
Gerrit-Change-Number: 5505 </div> <div style="display:none"> Gerrit-PatchSet: 1 \
</div> <div style="display:none"> Gerrit-Owner: Trung Nguyen \
&lt;trungnt282910@gmail.com&gt; </div> <div style="display:none"> Gerrit-MessageType: \
newchange </div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic