Re: [systemd-devel] [PATCH] localectl: support systems without locale-archive

2013-01-03 Thread Lennart Poettering
On Mon, 31.12.12 18:05, Giovanni Campagna (scampa.giova...@gmail.com) wrote:

 From: Giovanni Campagna gcampa...@src.gnome.org
 
 Not all systems ships with locales inside /usr/lib/locale-archive, some
 prefer to have locale data as individual subdirectories of /usr/lib/locale.
 (A notable example of this is OpenEmbeddded, and OSes deriving from it
 like gnome-ostree).
 Given that glibc supports both ways, localectl should too.

Sounds good. A few comments:

 +errno = 0;
 +while ((entry = readdir(dir))) {
 +char *z;
 +
 +if (errno != 0) {
 +log_error(Failed to read locale directory: %m);
 +r = -errno;
 +goto finish;
 +}

Hmm, this doesnt look right? readdir returns NULL on error, so this
check is misplaced?

 +
 +if (entry-d_type != DT_DIR)
 +continue;
 +
 +if (entry-d_name[0] == '.')
 +continue;

Please use ignore_file() here for this check.

 +z = strdup(entry-d_name);

We generally care for OOM in systemd. 

Hmm, I wonder if it might be nicer to add the entries from both places
to the set, and then only convert the set to a list once. Makes the code
shorter a bit... (I'd merge the patch anyway... this is just an idea...)

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] localectl: support systems without locale-archive

2012-12-31 Thread Giovanni Campagna
From: Giovanni Campagna gcampa...@src.gnome.org

Not all systems ships with locales inside /usr/lib/locale-archive, some
prefer to have locale data as individual subdirectories of /usr/lib/locale.
(A notable example of this is OpenEmbeddded, and OSes deriving from it
like gnome-ostree).
Given that glibc supports both ways, localectl should too.
---
 src/locale/localectl.c | 91 --
 1 file changed, 81 insertions(+), 10 deletions(-)

diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 383a17d..62de61b 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -266,7 +266,7 @@ finish:
 return r;
 }
 
-static int list_locales(DBusConnection *bus, char **args, unsigned n) {
+static int list_locales_from_archive(char ***l) {
 /* Stolen from glibc... */
 
 struct locarhead {
@@ -304,8 +304,6 @@ static int list_locales(DBusConnection *bus, char **args, 
unsigned n) {
 const struct namehashent *e;
 const void *p = MAP_FAILED;
 _cleanup_close_ int fd = -1;
-_cleanup_strv_free_ char **l = NULL;
-char **j;
 Set *locales;
 size_t sz = 0;
 struct stat st;
@@ -318,7 +316,8 @@ static int list_locales(DBusConnection *bus, char **args, 
unsigned n) {
 
 fd = open(/usr/lib/locale/locale-archive, 
O_RDONLY|O_NOCTTY|O_CLOEXEC);
 if (fd  0) {
-log_error(Failed to open locale archive: %m);
+if (errno != ENOENT)
+log_error(Failed to open locale archive: %m);
 r = -errno;
 goto finish;
 }
@@ -380,14 +379,89 @@ static int list_locales(DBusConnection *bus, char **args, 
unsigned n) {
 }
 }
 
-l = set_get_strv(locales);
-if (!l) {
+*l = set_get_strv(locales);
+if (!*l) {
 r = log_oom();
 goto finish;
 }
 
 set_free(locales);
 locales = NULL;
+r = 0;
+
+ finish:
+if (p != MAP_FAILED)
+munmap((void*) p, sz);
+
+set_free_free(locales);
+locales = NULL;
+return r;
+}
+
+static int list_locales_from_libdir (char ***l) {
+DIR *dir;
+Set *locales;
+struct dirent *entry;
+int r;
+
+locales = set_new(string_hash_func, string_compare_func);
+if (!locales)
+return log_oom();
+
+dir = opendir(/usr/lib/locale);
+if (!dir) {
+log_error(Failed to open locale directory: %m);
+r = -errno;
+goto finish;
+}
+
+errno = 0;
+while ((entry = readdir(dir))) {
+char *z;
+
+if (errno != 0) {
+log_error(Failed to read locale directory: %m);
+r = -errno;
+goto finish;
+}
+
+if (entry-d_type != DT_DIR)
+continue;
+
+if (entry-d_name[0] == '.')
+continue;
+
+z = strdup(entry-d_name);
+set_put(locales, z);
+}
+
+*l = set_get_strv(locales);
+if (!*l) {
+r = log_oom();
+goto finish;
+}
+
+set_free(locales);
+locales = NULL;
+r = 0;
+
+ finish:
+closedir(dir);
+set_free_free(locales);
+return r;
+}
+
+static int list_locales(DBusConnection *bus, char **args, unsigned n) {
+char **l;
+char **j;
+int r;
+
+l = NULL;
+r = list_locales_from_archive (l);
+if (r == -ENOENT)
+r = list_locales_from_libdir (l);
+if (r  0)
+goto finish;
 
 strv_sort(l);
 
@@ -399,10 +473,7 @@ static int list_locales(DBusConnection *bus, char **args, 
unsigned n) {
 r = 0;
 
 finish:
-if (p != MAP_FAILED)
-munmap((void*) p, sz);
-
-set_free_free(locales);
+strv_free(l);
 
 return r;
 }
-- 
1.8.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel