[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-gettext] [PATCH] intl: Add API to get/set language precedence l
From: |
Bruno Haible |
Subject: |
Re: [bug-gettext] [PATCH] intl: Add API to get/set language precedence list |
Date: |
Thu, 12 May 2016 19:28:52 +0200 |
User-agent: |
KMail/4.8.5 (Linux/3.8.0-44-generic; KDE/4.8.5; x86_64; ; ) |
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