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

Re: [PATCH] Generate locale files on request




--

  Martin Gracik

----- Original Message -----
> 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.

The scripts/* cannot be removed yet, the secondary architectures still use buildinstall,
we talked about this in Tempe.

> 
> Other than that I applaud those image-shrinking efforts.
> 
> Al_
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


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