On Mon, Sep 20, 2010 at 5:44 PM, Lennart Poettering
<lenn...@poettering.net> wrote:
> On Sun, 19.09.10 21:23, Gustavo Sverzut Barbieri (barbi...@profusion.mobi)
> wrote:
>
> Heya,
>
> Looks pretty good, but a few requests:
>
>> +#ifdef TARGET_GENTOO
>> +/* Gentoo uses sys-apps/kbd that installs to /usr and have fonts in
>> CamelCase */
>> +#define KBD_LOADKEYS "/usr/bin/loadkeys"
>> +#define KBD_SETFONT "/usr/bin/setfont"
>> +#define DEFAULT_FONT "LatArCyrHeb-16"
>> +#else
>> +#define KBD_LOADKEYS "/bin/loadkeys"
>> +#define KBD_SETFONT "/bin/setfont"
>> +#define DEFAULT_FONT "latarcyrheb-sun16"
>> +#endif
>
> I think it would be nicer if the path and filename configuration would
> happen in the Makefile, instead of in the sources. Could you move this
> to Makefile.am?
yes I could... but I fear getting an exponential number of configure
options... or you mean just the Makefile.am and not configure.ac? what
would be the configure.ac option name that you like?
>> +#elif defined(TARGET_GENTOO)
>> + if ((r = parse_env_file("/etc/rc.conf", NEWLINE,
>> + "unicode", &vc_unicode,
>> + NULL)) < 0) {
>> + if (r != -ENOENT)
>> + log_warning("Failed to read /etc/rc.conf: %s",
>> strerror(-r));
>> + }
>> + if (vc_unicode) {
>> + int rc_unicode;
>> +
>> + if (strcasecmp(vc_unicode, "yes") == 0 ||
>> + strcasecmp(vc_unicode, "on") == 0 ||
>> + strcasecmp(vc_unicode, "true") == 0 ||
>> + strcmp(vc_unicode, "1") == 0)
>> + rc_unicode = 1;
>
> Please use parse_boolean() here, which does pretty much exactly what you
> want to do here, but is much nicer to read.
well... not 100% as you just match the first letter of yes/true, but
yeah, who cares... I'll change that and send the new version.
--
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: barbi...@gmail.com
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
