[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] Generate locale files on request



Hi,

some notes below.

On 02/14/2011 10:10 AM, Martin Sivak wrote:
+    /* get locale parts out of locale name */
+    locale_p = locale_i = languages[choice].lc_all;

Could you please add a sample locale string or two as a comment, it makes the loop a lot easier to read and figure out what we really parse.

+    while (1) {
+        // we found separator
+        if (*locale_i == '.' || *locale_i == '@' || *locale_i == '\0') {
+            // last separator was annotating charset
+            if (*locale_p == '.') locale_charset = strndup(locale_p + 1, locale_i - locale_p - 1);
+            // last separator was annotating modifier
+            else if (*locale_p == '@') locale_mod = strndup(locale_p + 1, locale_i - locale_p - 1);
+            // there was no known separator last time
+            else locale_def = strndup(locale_p, locale_i - locale_p);

Doesn't this else imply locale_i == '\0' ?

+    /* generate locale record */
+    logMessage(INFO, "going to prepare locales for %s (locale: %s, charset: %s)", languages[choice].lc_all, locale_p, locale_charset);
+    if ((localedef_pid = fork()) == 0) {
+        execl("/usr/bin/localedef", "localedef",
+              "-i", locale_p,
+              "-f", (locale_charset) ? locale_charset : "UTF-8",
+              languages[choice].lc_all, NULL);
+    }
+    if (localedef_pid<  0) logMessage(ERROR, "failed starting localedef for %s", languages[choice].lc_all);

I think you want this log message say that a fork failed (unlikely, maybe just abort). Also there should be an assert/abort/return after the call to execl---that can fail but we don't want two instances of the process to continue.


diff --git a/scripts/upd-instroot b/scripts/upd-instroot
index 5c8d5b5..2b2fc4a 100755
--- a/scripts/upd-instroot
+++ b/scripts/upd-instroot

There was a pjones's patch around that would remove all this stuff so maybe this is not even necessary.

Other than that I applaud those image-shrinking efforts.

Al_


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]