Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 24 Jan 2010 04:18:48 +0900
From:      Hajimu UMEMOTO <ume@FreeBSD.org>
To:        Gabor Kovesdan <gabor@FreeBSD.org>
Cc:        Kostik Belousov <kostikbel@gmail.com>, attilio@FreeBSD.org, current@FreeBSD.org
Subject:   Re: NLS/strerror efficiency
Message-ID:  <yge1vhgvdd3.wl%ume@mahoroba.org>
In-Reply-To: <4B56CACF.50508@FreeBSD.org>
References:  <20100119212019.GL59590@deviant.kiev.zoral.com.ua> <4B56CACF.50508@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--Multipart_Sun_Jan_24_04:18:48_2010-1
Content-Type: text/plain; charset=ISO-2022-JP-2

Hi,

>>>>> On Wed, 20 Jan 2010 10:20:15 +0100
>>>>> Gabor Kovesdan <gabor@FreeBSD.org> said:

gabor> El 2010. 01. 19. 22:20, Kostik Belousov escribi$(D+Q(B:
> Hi,
> r189765 enabled NLS support for libc. Now, any strerror(3) call causes
> 4 (!) failing stat(2) calls. I think this is untolerable.
>
> Catopen() does not cache the catalog descriptor, at least for libc,
> at least for the case where the open failed.
>    
gabor> Hi Kostik,

gabor> thank you for pointing this out. I'll check the code to see how we could 
gabor> add a caching for the failing catalogs. I'll post the patch here when 
gabor> I'm done.

The gai_strerror(3) has same issue.  How about the attached patch in
this mail?

Sincerely,

--Multipart_Sun_Jan_24_04:18:48_2010-1
Content-Type: text/x-patch; type=patch; charset=US-ASCII
Content-Disposition: attachment; filename="strerror-lock.diff"
Content-Transfer-Encoding: 7bit

Index: lib/libc/include/libc_private.h
diff -u lib/libc/include/libc_private.h.orig lib/libc/include/libc_private.h
--- lib/libc/include/libc_private.h.orig	2010-01-01 02:14:04.000000000 +0900
+++ lib/libc/include/libc_private.h	2010-01-24 03:39:20.480514921 +0900
@@ -211,4 +211,10 @@
 /* execve() with PATH processing to implement posix_spawnp() */
 int _execvpe(const char *, char * const *, char * const *);
 
+#ifdef NLS
+extern char	*__cat_gets(int, int, const char *);
+extern void	__cat_lock(void);
+extern void	__cat_unlock(void);
+#endif
+
 #endif /* _LIBC_PRIVATE_H_ */
Index: lib/libc/net/gai_strerror.c
diff -u -p lib/libc/net/gai_strerror.c.orig lib/libc/net/gai_strerror.c
--- lib/libc/net/gai_strerror.c.orig	2009-11-23 02:05:46.000000000 +0900
+++ lib/libc/net/gai_strerror.c	2010-01-24 03:30:15.086490501 +0900
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD: src/lib/libc/net/gai
 #include "reentrant.h"
 #endif
 #include "un-namespace.h"
+#include "libc_private.h"
 
 /* Entries EAI_ADDRFAMILY (1) and EAI_NODATA (7) are obsoleted, but left */
 /* for backward compatibility with userland code prior to 2553bis-02 */
@@ -79,7 +80,6 @@ const char *
 gai_strerror(int ecode)
 {
 #if defined(NLS)
-	nl_catd catd;
 	char *buf;
 
 	if (thr_main() != 0)
@@ -98,17 +98,17 @@ gai_strerror(int ecode)
 		}
 	}
 
-	catd = catopen("libc", NL_CAT_LOCALE);
+	__cat_lock();
 	if (ecode > 0 && ecode < EAI_MAX)
-		strlcpy(buf, catgets(catd, 3, ecode, ai_errlist[ecode]),
+		strlcpy(buf, __cat_gets(3, ecode, ai_errlist[ecode]),
 		    sizeof(gai_buf));
 	else if (ecode == 0)
-		strlcpy(buf, catgets(catd, 3, NL_MSGMAX - 1, "Success"),
+		strlcpy(buf, __cat_gets(3, NL_MSGMAX - 1, "Success"),
 		    sizeof(gai_buf));
 	else
-		strlcpy(buf, catgets(catd, 3, NL_MSGMAX, "Unknown error"),
+		strlcpy(buf, __cat_gets(3, NL_MSGMAX, "Unknown error"),
 		    sizeof(gai_buf));
-	catclose(catd);
+	__cat_unlock();
 	return buf;
 
 thr_err:
Index: lib/libc/string/strerror.c
diff -u -p lib/libc/string/strerror.c.orig lib/libc/string/strerror.c
--- lib/libc/string/strerror.c.orig	2009-08-03 17:13:06.000000000 +0900
+++ lib/libc/string/strerror.c	2010-01-24 04:11:37.168662523 +0900
@@ -33,14 +33,16 @@ static char sccsid[] = "@(#)strerror.c	8
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD: src/lib/libc/string/strerror.c,v 1.16.10.1 2009/08/03 08:13:06 kensmith Exp $");
 
-#if defined(NLS)
-#include <nl_types.h>
-#endif
-
+#include "namespace.h"
 #include <limits.h>
 #include <errno.h>
 #include <string.h>
 #include <stdio.h>
+#if defined(NLS)
+#include <nl_types.h>
+#include "reentrant.h"
+#endif
+#include "un-namespace.h"
 
 #define	UPREFIX		"Unknown error"
 
@@ -52,6 +54,31 @@ __FBSDID("$FreeBSD: src/lib/libc/string/
  */
 #define	EBUFSIZE	(20 + 2 + sizeof(UPREFIX))
 
+#if defined(NLS)
+static mutex_t catd_mutex = MUTEX_INITIALIZER;
+static nl_catd catd;
+
+char *
+__cat_gets(int set_id, int msg_id, const char *s)
+{
+	if (catd == NULL)
+		catd = catopen("libc", NL_CAT_LOCALE);
+	return (catgets(catd, set_id, msg_id, s));
+}
+
+void
+__cat_lock(void)
+{
+	mutex_lock(&catd_mutex);
+}
+
+void
+__cat_unlock(void)
+{
+	mutex_unlock(&catd_mutex);
+}
+#endif
+
 /*
  * Doing this by hand instead of linking with stdio(3) avoids bloat for
  * statically linked binaries.
@@ -83,14 +110,14 @@ strerror_r(int errnum, char *strerrbuf, 
 	int retval = 0;
 #if defined(NLS)
 	int saved_errno = errno;
-	nl_catd catd;
-	catd = catopen("libc", NL_CAT_LOCALE);
+
+	__cat_lock();
 #endif
 
 	if (errnum < 1 || errnum >= sys_nerr) {
 		errstr(errnum,
 #if defined(NLS)
-			catgets(catd, 1, 0xffff, UPREFIX),
+			__cat_gets(1, 0xffff, UPREFIX),
 #else
 			UPREFIX,
 #endif
@@ -99,7 +126,7 @@ strerror_r(int errnum, char *strerrbuf, 
 	} else {
 		if (strlcpy(strerrbuf,
 #if defined(NLS)
-			catgets(catd, 1, errnum, sys_errlist[errnum]),
+			__cat_gets(1, errnum, sys_errlist[errnum]),
 #else
 			sys_errlist[errnum],
 #endif
@@ -108,7 +135,7 @@ strerror_r(int errnum, char *strerrbuf, 
 	}
 
 #if defined(NLS)
-	catclose(catd);
+	__cat_unlock();
 	errno = saved_errno;
 #endif
 

--Multipart_Sun_Jan_24_04:18:48_2010-1
Content-Type: text/plain; charset=US-ASCII


--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@mahoroba.org  ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/

--Multipart_Sun_Jan_24_04:18:48_2010-1--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?yge1vhgvdd3.wl%ume>