Hi Daiki,

The patch looks already nearly perfect to me. Just a few (minor) points:

- In function GET_I18N_LANGUAGE I would add a comment:

  /* No need to protect this access by
     gl_rwlock_rdlock (_nl_state_lock);
     because reading a single aligned memory word is atomic. */

  Rationale: Usually "protecting by a lock" means that all write
  accesses and all read accesses happen only while the lock is taken.
  If we do a read access without the lock being taken, it needs a
  justification.

- I would introduce another lock. The _nl_state_lock is meant to
  protect the variables of gettext() and friends - what I called the
  layer (C) in the earlier mail.

- I would move the definitions of GET_I18N_LANGUAGE and SET_I18N_LANGUAGE
  to a new .c file. They are in layer (B), whereas dcigettext.c is part
  of layer (C). Therefore they don't belong in the same .c file, IMO.
  Also I think this will make it easier to add a gnulib module that
  provides the get_i18n_language(), set_i18n_language() functions on
  systems that don't have the new glibc.

- Probably I would also create a new .h file for these two functions,
  because they are in layer (B) - like <locale.h> is also different from
  <libintl.h>. Yes I know gnulib has the techniques for overriding
  <libintl.h> when it needs to add two more function declarations to it;
  but nevertheless.

Bruno


Reply via email to