Hello,
This week I realised of a quite big mistake in the gettext module. Every
time that the user of this module was asking for string translation,
gettext module was allocating new memory and returning it. So if the
user was asking 10 times for the same string, gettext was creating 10
new strings and leaking memory.
With the attached patch, gettext module saves in a list the
translations. So if a user asks again for the same translation it
returns the same pointer to the already allocated string instead of a
new one.
If someone wants to discuss things maybe in the IRC is better (but feel
free to talk it here too). There are different approaches, and the
current one is not perfect but I don't see any perfect design.
Reviewers of the patch?
Thanks,
--
Carles Pina i Estany
http://pinux.info
2009-11-29 Carles Pina i Estany <car...@pina.cat>
* gettext/gettext.c: Include `<grub/list.h>'. Define grub_gettext_msg,
grub_gettext_msg_list.
(grub_gettext_gettranslation_from_position): Return static const char *
and not static char *
(grub_gettext_translate): Add the translated strings into a list,
returns from the list if existing there.
(grub_gettext_delete_list): Delete the list.
(grub_gettext_env_write_lang): Call grub_gettext_delete_list changes
the language.
(GRUB_MOD_FINI): Delete the list.
=== modified file 'gettext/gettext.c'
--- gettext/gettext.c 2009-11-24 21:42:14 +0000
+++ gettext/gettext.c 2009-11-29 00:41:29 +0000
@@ -17,6 +17,7 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <grub/list.h>
#include <grub/types.h>
#include <grub/misc.h>
#include <grub/mm.h>
@@ -33,7 +34,6 @@
http://www.gnu.org/software/autoconf/manual/gettext/MO-Files.html .
*/
-
static grub_file_t fd_mo;
static int grub_gettext_offsetoriginal;
@@ -41,6 +41,16 @@ static int grub_gettext_max;
static const char *(*grub_gettext_original) (const char *s);
+struct grub_gettext_msg
+{
+ struct grub_gettext_msg *next;
+ const char *name;
+
+ const char *translated;
+};
+
+struct grub_gettext_msg *grub_gettext_msg_list = NULL;
+
#define GETTEXT_MAGIC_NUMBER 0
#define GETTEXT_FILE_FORMAT 4
#define GETTEXT_NUMBER_OF_STRINGS 8
@@ -79,7 +89,7 @@ grub_gettext_getstring_from_offset (grub
translation[length] = '\0';
}
-static char *
+static const char *
grub_gettext_gettranslation_from_position (int position)
{
int offsettranslation;
@@ -130,9 +140,26 @@ static const char *
grub_gettext_translate (const char *orig)
{
char *current_string;
- char *ret;
+ const char *ret;
int min, max, current;
+ int found = 0;
+
+ struct grub_gettext_msg *cur;
+
+ cur = grub_named_list_find (GRUB_AS_NAMED_LIST (grub_gettext_msg_list),
+ orig);
+
+ if (!cur)
+ cur = grub_zalloc (sizeof (*cur));
+ else
+ return cur->translated;
+
+ if (!cur)
+ {
+ grub_errno = GRUB_ERR_NONE;
+ return orig;
+ }
if (fd_mo == 0)
return orig;
@@ -142,7 +169,7 @@ grub_gettext_translate (const char *orig
current = (max + min) / 2;
- while (current != min && current != max)
+ while (current != min && current != max && found == 0)
{
current_string = grub_gettext_getstring_from_position (current);
@@ -160,13 +187,26 @@ grub_gettext_translate (const char *orig
else if (grub_strcmp (current_string, orig) == 0)
{
grub_free (current_string);
- return grub_gettext_gettranslation_from_position (current);
+ found = 1;
}
current = (max + min) / 2;
}
- ret = grub_malloc (grub_strlen (orig) + 1);
- grub_strcpy (ret, orig);
+ ret =
+ found ? grub_gettext_gettranslation_from_position (current) :
+ grub_strdup (orig);
+
+ cur->name = grub_strdup (orig);
+ if (!cur->name)
+ {
+ grub_free (cur);
+ return orig;
+ }
+
+ cur->translated = ret;
+ grub_list_push (GRUB_AS_LIST_P (&grub_gettext_msg_list),
+ GRUB_AS_LIST (cur));
+
return ret;
}
@@ -259,12 +299,26 @@ grub_gettext_init_ext (const char *lang)
}
}
+static void
+grub_gettext_delete_list ()
+{
+ struct grub_gettext_msg *item;
+
+ while ((item =
+ grub_list_pop (GRUB_AS_LIST_P (&grub_gettext_msg_list))) != 0)
+ {
+ // Don't delete the translated message because could be in use.
+ }
+}
+
static char *
grub_gettext_env_write_lang (struct grub_env_var *var
__attribute__ ((unused)), const char *val)
{
grub_gettext_init_ext (val);
+ grub_gettext_delete_list ();
+
return grub_strdup (val);
}
@@ -307,5 +361,7 @@ GRUB_MOD_FINI (gettext)
if (fd_mo != 0)
grub_file_close (fd_mo);
+ grub_gettext_delete_list ();
+
grub_gettext = grub_gettext_original;
}
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
