Re: GLib patch

[Owen Taylor <otaylor redhat com>]

Andrew Taylor <andrew taylor montage ca> writes:

> Oops, I guess I missed the big "This file is automatically
> generated. DO NOT EDIT!" at the top of the files.  Here is a new patch
> that also changes the script that generates these files.
> 
> In addition, it const-izes some data I missed in the first patch.
> This one moves about 49k into .rodata.
> 
> Andrew
> 
> Andrew Taylor wrote:
> 
> > Here is a patch to glib that makes about 27k of static unicode
> > attribute data "const" so it can be thrown into the .rodata section
> > rather than the .data section when compiled with gcc.  This is good
> > because .rodata is always shared in shared libraries.  The .data
> > section not always shared -- it is copy on write with entire pages
> > (usually 4k) duplicated at a time.
> > That, and it might catch some future coding errors.
> > My patch is against 1.3.8.  Let me know if this is the wrong mailing
> > list for such a patch!
> > Andrew

Thanks for a patch. I've applied it now.

A few notes:

 - Actually the .rodata section is not always shared. Relocations
   can force copies to be made. Luckily (and intentionally)
   most of the design of the unicode tables is to prevent this ...
   that's why we have:

    static const guchar special_case_table[][18] = {
  
   Instead of:

    static const guchar * const special_case_table[] = {

   More could be done in this regard ... in general, all the
   "page tables" should be done not as pointers to multiple
   subarrays:
  
    static const unsigned short *attr_table[256] = {
      attrpage0,
      attrpage1,
      attrpage2,
      NULL,

   But as indicies into one large table:

    static const char attr_table[256] = {
      0,
      1
      2,
      -1

   Or something like that. But that would take a fair bit of
   rewriting of gen-unicode-tables.pl.

   (The number of fixups for the unicode tables in glib is close
   to the number in the rest of the gtk+ libraries total....)

 - It would be better to simply send the diff to 
   gen-unicode-tables.pl. What I did was verified that all the
   changes in the tables in the patch were in gen-unicode-tables.pl 
   patch, applied the gen-unicode-tables.pl diff and regenerated
   the headers.

 - Patches are best submitted to bugzilla.gnome.org (details
   in the README.) Otherwise, there is a very high probability
   that they will be forgotten.

 - We would generally do 'static const char' not 
   'const static char' since 

Regards,
                                        Owen