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

[Lennart Poettering]

On Mon, 31.12.12 18:05, Giovanni Campagna (scampa.giovanni at gmail.com) wrote:

> From: Giovanni Campagna <gcampagna at 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.