Re: [PATCH] Generate locale files on request

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

 



Ack, thanks.

Al_

On 02/14/2011 03:40 PM, 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.

I can, but for the review the formats are as follows:

en
en_US
cs_CZ.UTF-8
cz_CZ.UTF-8@latin

+ 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' ?

Nope, the first if tests current character, but following ifs test locale_p (the last separator), so the else is actually processed first, because locale_p points to first character of locale.


+ /* 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.



True, exit should be there. I will add one :)


--
Martin SivÃk
msivak@xxxxxxxxxx
Red Hat Czech
Anaconda team / Brno, CZ

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list



[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux