On Sat, Nov 21, 2009 at 10:14:10PM +0000, Carles Pina i Estany wrote:
> @@ -179,7 +185,7 @@
> pkglib_MODULES += fshelp.mod fat.mod ufs1.mod ufs2.mod ext2.mod ntfs.mod \
> ntfscomp.mod minix.mod hfs.mod jfs.mod iso9660.mod xfs.mod \
> affs.mod sfs.mod hfsplus.mod reiserfs.mod cpio.mod tar.mod \
> - udf.mod afs.mod afs_be.mod befs.mod befs_be.mod
> + udf.mod afs.mod afs_be.mod befs.mod befs_be.mod gettext.mod
>
> # For fshelp.mod.
> fshelp_mod_SOURCES = fs/fshelp.c
> @@ -610,6 +616,11 @@
> bufio_mod_CFLAGS = $(COMMON_CFLAGS)
> bufio_mod_LDFLAGS = $(COMMON_LDFLAGS)
>
> +# For gettext.mod.
> +gettext_mod_SOURCES = gettext/gettext.c
> +gettext_mod_CFLAGS = $(COMMON_CFLAGS)
> +gettext_mod_LDFLAGS = $(COMMON_LDFLAGS)
I haven't switched the old declarations, but for new modules please use
something like:
pkglib_MODULES += gettext.mod
gettext_mod_SOURCES = gettext/gettext.c
gettext_mod_CFLAGS = $(COMMON_CFLAGS)
gettext_mod_LDFLAGS = $(COMMON_LDFLAGS)
this way it is easier to move declarations around, etc.
> +static grub_file_t grub_mofile_open (const char *name);
We usually skip declarations for static functions (when all its users are
placed after its implementation, of course).
> +#define GETTEXT_MAGIC_NUMBER 0
> +#define GETTEXT_FILE_FORMAT 4
> +#define GETTEXT_NUMBER_OF_STRINGS 8
> +#define GETTEXT_OFFSET_ORIGINAL 12
> +#define GETTEXT_OFFSET_TRANSLATION 16
These would look much prettier with some tabs ;-)
> + grub_file_seek (fd_mo, offset);
> + grub_file_read (fd_mo, translation, length);
You do a lot of grub_file_seek + grub_file_read throurough the code. Maybe
grub_file_pread() would be more appropiate? (for space saving)
> + grub_free(current_string);
> + min=current;
> [...]
> + current = (max+min)/2;
> [...]
> + ret = grub_malloc(grub_strlen(orig) + 1);
> + grub_strcpy(ret,orig);
Missing spaces (between function name and parenthesis, around '=', etc).
I suggest you use indent(1), this will automatically adjust to our coding
style.
> + /*
> + Do we want .mo.gz files? Then, the code:
> + file = grub_gzio_open (io, 0); // 0: transparent
> + if (! file)
> + {
> + grub_printf("Problems opening the file\n");
> + grub_file_close (io);
> + return 0;
> + }
> + */
Uhm I wonder if we could answer this question before the code is merged; what
does everyone think about .mo.gz?
> + locale_dir = grub_env_get ("locale_dir");
This needs to be checked for NULL.
> + // mo_file e.g.: /boot/grub/locale/ca.mo
Please use C-style comments (/* */).
> + mo_file = grub_malloc (grub_strlen (locale_dir) + sizeof ("/") +
> grub_strlen (lang) + sizeof(".mo"));
Note that sizeof ("/") equals 2 ('/' + '\0').
> + grub_dprintf("gettext", "Will try to open file: %s " ,mo_file);
> +
> + fd_mo = grub_mofile_open(mo_file);
In this case I think error handling would be more useful than debug print (i.e.
if file can't be opened, rise an error).
> + /* Testing:
> + grub_register_command ("_", grub_cmd_translate, GRUB_COMMAND_FLAG_BOTH,
> + "_", "internalization support trans", 0);
> + */
We could define this interface unconditionally, but it'd be more consistent
as `gettext' command (like the one in GNU gettext package).
> === added file 'include/grub/i18n_grub.h'
> --- include/grub/i18n_grub.h 1970-01-01 00:00:00 +0000
> +++ include/grub/i18n_grub.h 2009-11-19 21:32:05 +0000
> [...]
> +#ifndef GRUB_I18N_GRUB_H
> +#define GRUB_I18N_GRUB_H 1
> +
> +# define _(str) grub_gettext(str)
> +
> +#endif /* GRUB_I18N_GRUB_H */
You can use <grub/i18n.h> for this (in fact it already defines _ to a
noop stub for non-util and would just need to be modified).
> +const char *EXPORT_FUNC(grub_gettext_dummy) (const char *s);
> +extern const char *(*EXPORT_VAR(grub_gettext)) (const char *s);// =
> grub_gettext_dummy;
This could be in i18n.h too (except the comment ;-)).
Btw, is it necessary to export grub_gettext_dummy symbol? If we avoid it
it's a few less bytes in kernel :-)
> +# Gettext variables and module
> +cat << EOF
> +set locale_dir=${locale_dir}
> +set lang=${grub_lang}
> +insmod gettext
> +EOF
We could avoid loading the module at all for non-utf8 capable setups (when
this happens, grub-mkconfig exports LANG=C, so this can be easily detected).
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
