[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <at0K3-wjGF8SE_D5@mail.gmail.com>
Date: Sat, 24 Oct 2026 21:45:35 +0200
From: Luca Kellermann <mailto.luca.kellermann@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH v2] musl locale overhaul series, take 2
On Fri, Oct 02, 2026 at 01:38:00AM -0400, Rich Felker wrote:
> The attached patch series is the current contents of my integration
> branch for the locale support overhaul project, funded by NLnet and
> the NGI Zero Core Fund. Compared to the v1 draft, this is now largely
> in the form it will be merged to the master branch after the 1.2.7
> bugfix release. Significant additional work has been done to eliminate
> most of the increased size of global data for locale, and to reduce
> code churn at the same time.
>
> As before these are factored into small commits to admit individual
> assessment both now and in the historical record. The high-level
> changes are:
I reviewed most changes in detail and found a few bugs, see below. I
also suggest some other changes.
I didn't review the logic for collation and strfmon().
> - Updates of built-in character data to Unicode 17. This is entirely
> mechanical, generated by musl-chartable-tools[1], probably not
> intersting to read. The bugs present in the v1 patch series due to
> parse errors during table generation are now fixed.
Unicode 18 is available since september.
> [...]
>
> - Linear-search-free versions of all of the error message lookup
> interfaces (strerror, hstrerror, gai_strerror, regerror), signal
> name lookup (strsignal), nl_langinfo (this is the backend for
> localized data/time formatting), and gettext error messages. These
getopt, not gettext.
> now all use the new string table lookup infrastructure for both
> built-in and locale string tables.
>
> [...]
>
> From 0f52112210365e9cf9ee139c1bdcfa1ce4dc887c Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Sat, 12 Sep 2026 18:02:56 -0400
> Subject: [PATCH 03/23] add integer-keyed multi-level table lookup for new
> locale support
>
> [...]
>
> diff --git a/src/internal/ikmlt.c b/src/internal/ikmlt.c
> [...]
> +/* ikmlt_lookup_path takes a path of keys (first element of path is
> + * the number of components) starting from the tabel ld and traversing
Typo: tabel
> [...]
> diff --git a/src/internal/ikmlt.h b/src/internal/ikmlt.h
> [...]
> +#define ikmlt_lookup __ikmlt_lookup
> +#define ikmlt_lookup_path __ikmlt_lookup_path
> +
> +hidden const void *ikmlt_lookup(const unsigned char *, int);
> +hidden const void *ikmlt_lookup_path(const unsigned char *, const int *);
Are these defines really worthwhile just to safe a few characters?
> [...]
>
> From 42c57b8d8505d79c4cba540a1d137999a95f5b53 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Sat, 12 Sep 2026 20:41:12 -0400
> Subject: [PATCH 04/23] add common __loc_lookup backend for new locale
>
> [...]
>
> diff --git a/src/locale/__loc_lookup.c b/src/locale/__loc_lookup.c
> [...]
> +const char *__loc_lookup(const unsigned char *ptable, const unsigned char *ltable, const int *path, const int *fallback)
> +{
Changing the type of ptable to const void * would remove the need for
a cast at every callsite.
> + if (!path) return 0;
> + const char *pstr = ikmlt_lookup_path(ptable, path);
> + if (!pstr) return __loc_lookup(ptable, ltable, fallback, 0);
> + if (!ltable) return (char *)pstr;
> + const char *lstr = ikmlt_lookup_path(ltable, path);
> + return lstr ? (char *)lstr : (char *)pstr;
The casts to char * seem unnecessary.
> [...]
>
> From deba1b9ed3762e79c232ebf9df6e72cbe79b8fc4 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Sat, 12 Sep 2026 22:24:28 -0400
> Subject: [PATCH 05/23] add include frameworks for building built-in C locale
> string tables
>
> [...]
>
> diff --git a/src/internal/mdata.h b/src/internal/mdata.h
> [...]
> +#define M_3(a,b,c,...) c
> +#define M_S(...) M_3(,,__VA_ARGS__)
> +#define M_N(...) M_3(,__VA_ARGS__)
> +#define M_B(...) M_3(__VA_ARGS__)
M_B() should be defined as M_3(__VA_ARGS__,). Otherwise you get the
following with -pedantic: warning: ISO C99 requires at least one
argument for the "..." in a variadic macro
> +.header = {
> + 0xff & (M_B(M_TABLE)>>24),
> + 0xff & (M_B(M_TABLE)>>16),
> + 0xff & (M_B(M_TABLE)>>8),
> + 0xff & (M_B(M_TABLE)),
M_B(M_TABLE) will sometimes be a negative int. Shifting negative
values is implementation-defined. Maybe convert to unsigned before?
> + 0, 1,
> + (sizeof (unsigned char[]){
char would suffice. The array is only used to count.
> +#define M(n, s) [(n)-(M_B(M_TABLE))] = 0,
> +#include M_S(M_TABLE)
> +#undef M
> + } - 1) >> 8,
> + (sizeof (unsigned char[]){
> +#define M(n, s) [(n)-(M_B(M_TABLE))] = 0,
> +#include M_S(M_TABLE)
> +#undef M
M() is identically defined twice here, maybe just define it once?
> + } - 1) & 0xff,
> +},
> +.offsets = {
> +#define M(n, s) \
> + [2*((n)-(M_B(M_TABLE)))] = \
> + (offsetof(struct M_N(M_TABLE), m_##n)+1)/256,\
> + (offsetof(struct M_N(M_TABLE), m_##n)+1)%256,
Otherwise this file uses >> and & instead of / and %.
> [...]
> diff --git a/src/internal/mdecl.h b/src/internal/mdecl.h
> [...]
> +#define M_3(a,b,c,...) c
> +#define M_S(...) M_3(,,__VA_ARGS__)
> +#define M_N(...) M_3(,__VA_ARGS__)
> +#define M_B(...) M_3(__VA_ARGS__)
Same here with M_B().
> [...]
>
> From 0fdc2d64e40c52726fe216a48d5496448b41e935 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Sat, 12 Sep 2026 23:40:38 -0400
> Subject: [PATCH 06/23] switch langinfo and error strings over to new common
> table lookup
>
> [...]
>
> errno numbers can vary by arch (only mips at present), but the keys
powerpc (like mips) also has one additional error: EDEADLOCK (not the
same as EDEADLK like on other archs), but see my comments below.
> used in the table need to be arch-agnostic, since locale files may be
> shared across archs. the common arch/generic numbering is adopted,
> with a translation table built conditionally for archs that don't
> match. for now, EDEADLK != L_EDEADLK is used as a proxy condition; it
> can be expanded or explicitly declared by the arch if needed for
> future archs.
If my other comments about EDEADLK and EDEADLOCK are followed, it
might be worth considering to change the proxy condition (e.g. to
ENOMSG != L_ENOMSG) to avoid confusion. There are two concerns here:
completely different numbering (only mips) and whether EDEADLK and
EDEADLOCK are distinct errors (mips and powerpc).
> [...]
>
> diff --git a/src/errno/__strerror.h b/src/errno/__strerror.h
> [...]
> * with these messages, and then to define a lookup table translating
> * error codes to offsets of corresponding fields in the structure. */
This comment seems a bit out of date now, the lookup table and
structure containing the messages are defined together at the same
time.
> [...]
> diff --git a/src/errno/l_errno.h b/src/errno/l_errno.h
> [...]
> +#define L_EAGAIN 11
> [...]
> +#define L_EWOULDBLOCK 41
Why L_EAGAIN != L_EWOULDBLOCK? All supported archs have EAGAIN ==
EWOULDBLOCK. Is L_EWOULDBLOCK needed at all?
This even leads to the problem that archs using remap_errno.h will
return "Unknown error" from strerror(EAGAIN). The reason is that
remap_errno looks like this:
static unsigned char remap_errno[] = {
// ...
[EAGAIN] = L_EAGAIN, // [11] = 11,
// ...
[EWOULDBLOCK] = L_EWOULDBLOCK, // [11] = 41,
// ...
};
So remap_errno[11] will give 41 for which __strerror.h has no entry
(no E(EWOULDBLOCK, "...")). Removing L_EWOULDBLOCK and its use in
remap_errno.h would solve this. Comments in place of L_EWOULDBLOCK for
l_errno.h and remap_errno.h could clarify why it's missing:
/* L_EWOULDBLOCK not needed, EWOULDBLOCK == EAGAIN */
/* L_EWOULDBLOCK does not exist */
A similar problem could arise for a future arch where remap_errno.h is
used but where EDEADLK == EDEADLOCK. This could be solved by only
defining and using L_EDEADLOCK conditionally (similar to how
strsignal() handles SIGSTKFLT and SIGEMT):
l_errno.h:
#if EDEADLK != EDEADLOCK
#define L_EDEADLOCK 58
#endif
remap_errno.h:
#ifdef L_EDEADLOCK
[EDEADLOCK] = L_EDEADLOCK,
#endif
__strerror.h (this message does not exist yet but could be added):
#ifdef L_EDEADLOCK
// see errno(3)
E(EDEADLOCK, "File locking deadlock error")
#endif
If a message for EDEADLOCK would instead be added unconditionally, all
archs other than mips would return it for strerror(58), even though
only powerpc has EDEADLOCK defined as 58. Other archs should return
"Unknown error".
> [...]
> diff --git a/src/errno/remap_errno.h b/src/errno/remap_errno.h
> [...]
> +[EOPNOTSUPP] = L_EOPNOTSUPP,
This could be [ENOTSUP] = L_ENOTSUP. Then L_EOPNOTSUPP could be
removed from l_errno.h since __strerror.h uses L_ENOTSUP via
E(ENOTSUP, "Not supported"). Defining both L_ENOTSUP and L_EOPNOTSUPP
is a bit redundant, they have the same value. Comments like those I
suggested above for L_EWOULDBLOCK would be nice.
> [...]
> diff --git a/src/errno/strerror.c b/src/errno/strerror.c
> [...]
> -#include <string.h>
strerror() and strerror_r() are declared in string.h, so it should
probably stay included.
> [...]
> +enum { L_E0 = 0, L_E_ = -1 };
Could L_E0 and L_E_ be defined in l_errno.h instead?
> +#define M_TABLE "../errno/__strerror.h", strerror_data, -1
Shouldn't M_TABLE definitions use L_E_, EAI_OVERFLOW, H_, REG__, NEG1
and GETOPT_INVAL instead of magic numbers for M_B()?
> +#define ERRDOMAIN_ERRNO 0
> +
> +static const struct {
> + unsigned char header[8];
> + unsigned char offsets[4];
> + struct {
> + #include "mdecl.h"
> + } errors;
> +} strerrors = {
> + .header = { 0,0,0,0,0,0,0,4-1, },
> + .offsets = { [ERRDOMAIN_ERRNO] = 1, },
> + .errors = {
> + #include "mdata.h"
> + },
> +};
To save a few more bytes, these definitions could be written like
this (use ERRDOMAIN_x as start to only have a single offset):
static const struct {
unsigned char header[8];
unsigned char offsets[1];
struct {
#include "mdecl.h"
} errors;
} strerrors = {
.header = { 0,0,0,ERRDOMAIN_ERRNO },
.offsets = { 1 },
.errors = {
#include "mdata.h"
},
};
Alignment shouldn't matter. The values are strings only and ikmlt
reads the headers and offsets in an alignment-agnostic way.
Repeat for all other ERRDOMAIN_s.
> [...]
> +static int common_errno(int e)
> {
> - const char *s;
> +#if EDEADLK != L_EDEADLK
> + static unsigned char remap_errno[] = {
> + #include "remap_errno.h"
> + };
> [...]
> + if (e >= sizeof remap_errno / sizeof *remap_errno) return -1;
/ sizeof *remap_errno can be omitted, it's always 1. This line would
also be clearer if it returned L_E_: conceptionally common_errno()
maps Ex to L_Ex and should return L_E_ for unknown values.
> + return remap_errno[e];
common_errno() should return L_E0 if !e and L_E_ if !remap_errno[e].
Right now strerror(47) returns "No error information" instead of
"Unknown error" on mips.
static int common_errno(int e)
{
#if ...
static unsigned char remap_errno[] = {
#include "remap_errno.h"
};
if (!e) return L_E0;
#ifdef EDQUOT_ORIG
if (e==EDQUOT) e=-1;
else if (e==EDQUOT_ORIG) e=EDQUOT;
#endif
if (e >= sizeof remap_errno) return L_E_;
e = remap_errno[e];
return e ? e: L_E_;
#else
return e;
#endif
}
> +#else
> + return e;
> +#endif
> +}
> +
> +char *__strerror_l(int e, locale_t loc)
> +{
> + e = common_errno(e);
> + static const int unknown[] = LPATH(ERRDOMAIN_ERRNO, -1);
The unknown path should probably use L_E_ instead of -1.
> [...]
> diff --git a/src/locale/langinfo.c b/src/locale/langinfo.c
> [...]
> +#define M_TABLE_TIME "../locale/lc_time.h", lc_time_data, LC_TIME<<16
> +#define M_TABLE_MESSAGES "../locale/lc_messages.h", lc_messages_data, LC_MESSAGES<<16
> +#define M_TABLE_MONETARY "../locale/lc_monetary.h", lc_monetary_data, LC_MONETARY<<16
> +#define M_TABLE_NUMERIC "../locale/lc_numeric.h", lc_numeric_data, LC_NUMERIC<<16
LC_MONETARY (4) could start at CRNCYSTR (0x4000F).
> +static const struct {
> + unsigned char header[8];
> + unsigned char offsets[12];
> + struct langinfo_data {
> + int :0;
What's the purpose of these zero-width bit-fields? Alignment? This
might be unnecessary, see my other comment about alignment.
> [...]
> + } data;
> +} c_langinfo = {
> + .header = { 0,0,0,0,0,1,0,6-1, },
> + .offsets = {
> + O(LC_TIME, lc_time),
> + O(LC_MESSAGES, lc_messages),
> + O(LC_MONETARY, lc_monetary),
> + O(LC_NUMERIC, lc_numeric),
> + },
> + .data = {
> [...]
> + },
> +};
If LC_NUMERIC (1) would be used as start, 2 bytes could be saved (or 4
if LC_NUMERIC is removed and LC_TIME (2) is used as start, see below):
static const struct {
unsigned char header[8];
unsigned char offsets[5*2]; // or 4*2
struct langinfo_data {
// ...
} data;
} c_langinfo = {
.header = { 0,0,0,1,0,1,0,5-1 }, // or 4-1
.offsets = {
// or -2 each
O(LC_TIME-1, lc_time),
O(LC_MESSAGES-1, lc_messages),
O(LC_MONETARY-1, lc_monetary),
O(LC_NUMERIC-1, lc_numeric),
},
.data = {
// ...
},
};
> char *__nl_langinfo_l(nl_item item, locale_t loc)
> {
> int cat = item >> 16;
> int idx = item & 65535;
> - const char *str;
> + int path[] = { 2, cat, item };
Why isn't LPATH() used here?
> + static const int unknown[] = { 2, -1, -1 };
This path doesn't lead to any value, so a null pointer could be passed
to __loc_lookup() instead.
> if (item == CODESET) return loc->cat[LC_CTYPE] ? "UTF-8" : "ASCII";
> + //if (item == RADIXCHAR && loc->cat[LC_NUMERIC])
> + // return loc->cat[LC_NUMERIC]->lconv.decimal_point;
RADIXCHAR and THOUSEP could be implemented like this to avoid
redundancy in the file format:
if (item == RADIXCHAR) {
static const int r_path[] = LPATH(K_LCONV, 0);
const char *str = __ikmlt_lookup_path(
LMEMB(loc, LC_NUMERIC, table_root), r_path);
if (!str || str[0] != ',' || str[1]) str = ".";
return (char *)str;
}
if (item == THOUSEP) {
static const int t_path[] = LPATH(K_LCONV, 1);
const char *str = __ikmlt_lookup_path(
LMEMB(loc, LC_NUMERIC, table_root), t_path);
return str ? (char *)str : "";
}
lc_numeric.h could then be removed and LC_TIME (2) could be used as
start in c_langinfo.
Is it possible to synthesize CRNCYSTR from currency_symbol,
p_cs_precedes and n_cs_precedes (or the other way around)? If so, I
have some ideas how to get rid of it from the file format.
> /* _NL_LOCALE_NAME extension */
> - if (idx == 65535 && cat < LC_ALL)
> + if (idx == 65535 && (unsigned)cat < LC_ALL)
Instead of casting to unsigned here, maybe cast item to unsigned
before shifting (shifting negative values is implementation-defined).
> [...]
> + const char *str = __loc_lookup((const void *)&c_langinfo,
> + 0 /* LMEMB(CURRENT_LOCALE,cat,langinfo) */,
> + path, unknown);
This should use loc instead of CURRENT_LOCALE. There will also be an
out of bounds access to loc->cat if cat < 0 or cat >= LC_ALL.
> [...]
> diff --git a/src/locale/lc_messages.h b/src/locale/lc_messages.h
> [...]
> +M(YESEXPR, "^[yY]\0")
> +M(NOEXPR, "^[nN]\0")
\0 can be omitted now.
> [...]
> diff --git a/src/locale/lc_time.h b/src/locale/lc_time.h
> [...]
> +M(D_T_FMT, "%a %b %e %T %Y")
I think this should be M(D_T_FMT, "%a %b %e %H:%M:%S %Y"), see
POSIX.1-2024 XBD 7.3.5.3 LC_TIME Category in the POSIX Locale [1].
> [...]
> diff --git a/src/regex/regerror.c b/src/regex/regerror.c
> [...]
> +enum { REG__ = -1 };
> +#define M_TABLE "../regex/regerror.h", regerror_data, -1
REG_ENOSYS is defined as -1 too, so adding a message for it will be
more work than just adding M(REG_ENOSYS, "...") to regerror.h. Using
-2 for REG__ would make it easier.
> [...]
>
> From 0cfd41aab32b1c7e7eb93c28f8582dfa1586106d Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Mon, 14 Sep 2026 17:44:45 -0400
> Subject: [PATCH 08/23] locale: remove synthesis of nonexistent locale names
>
> this change does not make setlocale fail for locale names lacking a
> definition, but it eliminates the behavior of preserving the requested
> name, instead handling them as aliases for C.UTF-8 (LC_CTYLE) or C
> (all other categories).
Typo: LC_CTYLE
> this breaks gettext search for translations to languages where no libc
> locale exists, but adding support for the new LANGUAGE environment
> variable will mitigate that.
Will this be added as part of the locale merge?
> [...]
>
> From 27359091d7126f0edaa30796eb102596446eacdb Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Mon, 14 Sep 2026 18:09:27 -0400
> Subject: [PATCH 09/23] locale: make explicit request of unavailable locales
> fail
>
> have __get_locale track whether the request is implicit (obtained via
> environment for an argument of "") or explicit. if explicit, names
> that fail to resove to a locale file will produce a return value of
> LOC_MAP_FAILED, which setlocale and newlocale already interpret as an
> error condition. if implicit, return C.UTF-8 for LC_CTYPE, and
> otherwise a null pointer (interpreted as C locale).
setlocale(LC_ALL, "some-very-very-very-very-very-very-long-name") will
still not fail but behave as setlocale(LC_ALL, "C.UTF-8"). setlocale()
could be changed like this:
char *setlocale(int cat, const char *name)
{
// ...
if (cat == LC_ALL) {
int i;
if (name) {
struct __locale_struct tmp_locale;
const char *p = name;
for (i=0; i<LC_ALL; i++) {
const char *z = __strchrnul(p, ';');
if (z-p > LOCALE_NAME_MAX) {
UNLOCK(__locale_lock);
return 0;
}
char part[LOCALE_NAME_MAX+1];
memcpy(part, p, z-p);
part[z-p] = 0;
if (*z) p = z+1;
lm = __get_locale(i, part);
// ...
}
// ...
}
// ...
}
// ...
}
> [...]
>
> diff --git a/src/locale/locale_map.c b/src/locale/locale_map.c
> [...]
> +fail:
> + if (!implicit) return LOC_MAP_FAILED;
As __get_locale() can now return LOC_MAP_FAILED, it must ensure that
errno is set to an appropriate value so newlocale() can pass it on.
This is currently not the case if called with an invalid locale name
(leading dot, slash, too long) or if the path search doesn't actually
try to map any file. Could be solved like this:
const struct __locale_map *__get_locale(int cat, const char *val)
{
// ...
int err = EINVAL; // or ENOENT, I'm not sure
if (val[0]=='.' || val[n])
goto fail;
// ...
err = ENOENT;
if (path) for (; *path; path=z+!!*z) {
// ...
const void *map = __map_file(buf, &map_size);
if (map) {
new = malloc(sizeof *new);
if (!new) {
err = errno;
__munmap((void *)map, map_size);
break;
}
// ...
break;
}
err = errno;
}
if (new) return new;
fail:
if (!implicit) {
errno = err;
return LOC_MAP_FAILED;
}
// ...
}
> [...]
>
> From 9b7e95d577305f6142e3c28273c973cfd586680a Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Tue, 15 Sep 2026 19:01:33 -0400
> Subject: [PATCH 10/23] locale: add loader for new locale format & adjust
> related logic
>
> [...]
>
> diff --git a/src/internal/locale_impl.h b/src/internal/locale_impl.h
> [...]
> + K_ERRORS,
> [...]
> + const unsigned char *errors;
This also includes strings for strsignal() and getopt(). These are not
really errors. Would K_STRINGS/strings/STRDOMAIN_x be better names?
> [...]
> diff --git a/src/locale/c_locale.c b/src/locale/c_locale.c
> [...]
> #include <stdint.h>
>
> -static const uint32_t empty_mo[] = { 0x950412de, 0, -1, -1, -1 };
This no longer needs to include stdint.h.
> [...]
> diff --git a/src/locale/locale_map.c b/src/locale/locale_map.c
> [...]
> +#include <stddef.h>
Why include stddef.h?
> [...]
> +static int load_locale(struct __locale_map *l, const unsigned char *lm)
> +{
> + const unsigned char *root;
> +
> + if (memcmp(lm, "ikmlt\xff\xc0\xc1loc2\0\0\0\0", 16)) {
Does it make sense to ensure that map_size > (16 + 8) (magic + header
of root table) before?
I'm also curious. Does this magic value have a meaning?
> + errno = EINVAL;
> + return -1;
> + }
Is EINVAL the right error for newlocale() in this situation? Or would
ENOENT (or some other error) be more appropriate?
> + l->table_root = root = lm+16;
> +
This empty line has a trailing tab.
> [...]
> const struct __locale_map *__get_locale(int cat, const char *val)
> {
> [...]
> + struct __locale_map tmp;
> + if (load_locale(&tmp, map) ||
> + !(new = malloc(sizeof *new))) {
> __munmap((void *)map, map_size);
> break;
> }
Couldn't the path search continue when seeing an invalid file
(load_locale() returns -1)?
> - new->map = map;
> - new->map_size = map_size;
> + *new = tmp;
> memcpy(new->name, val, n);
> new->name[n] = 0;
> new->next = loc_head;
Writing it this way would avoid copying a partially uninitialized tmp
(even when dummy_collation_init() is used later):
struct __locale_map tmp = { 0 };
if (load_locale(&tmp, map) ||
!(new = malloc(sizeof *new))) {
// ...
break;
}
memcpy(tmp.name, val, n);
tmp.name[n] = 0;
tmp.next = loc_head;
*new = tmp;
loc_head = new;
> [...]
>
> From 5ddebacde40d9f0150cd22ce8066a1ae7d6f90fd Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Mon, 21 Sep 2026 18:27:14 -0400
> Subject: [PATCH 13/23] convert strsignal to use new locale string table
> lookups
>
> [...]
>
> diff --git a/src/string/strsignal.c b/src/string/strsignal.c
> [...]
> #include <string.h>
> #include "locale_impl.h"
This file doesn't compile at this point in the patch series because it
isn't including stddef.h. It will compile with commit
aba708364983906ee36b1842fb1dc62dd157eefd because then locale_impl.h
includes collate.h which includes nfd.h which includes stddef.h.
> +enum { NEG1 = -1 };
This could either have a better name (e.g. L_SIGEMT) or no name at all
like the rest of the values.
> [...]
> char *strsignal(int signum)
> {
> - const char *s = strings;
> -
> - signum = sigmap(signum);
> - if (signum - 1U >= _NSIG-1) signum = 0;
> -
> - for (; signum--; s++) for (; *s; s++);
> + static const int unknown[] = LPATH(ERRDOMAIN_SIGNAL, 0);
> + const int path[] = LPATH(ERRDOMAIN_SIGNAL, signum);
This should still use sigmap(). Right now strsignal() is broken on
mips.
> [...]
> diff --git a/src/string/strsignal.h b/src/string/strsignal.h
> [...]
> +#ifdef SIGSTKFLT
> +M(16, "Stack fault")
> +#endif
> [...]
> +#if _NSIG > 65
> [...]
> +M(127, "RT127")
> +#endif
RT128 was previously included in the multistring. But it was never
used because 128 - 1U >= _NSIG-1 (_NSIG is 65 or 128). So you
intentionally left it out, right?
> +#ifdef SIGEMT
> +M(NEG1, "Emulator trap")
> +#endif
I would expect to find the message for SIGEMT next to SIGSTKFLT to
mimick strsignal.c.
> [...]
>
> From 4df51353e0739882b28c55c1fe4a451425539841 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Mon, 21 Sep 2026 20:05:18 -0400
> Subject: [PATCH 14/23] convert getopt to use new locale string table lookups
>
> [...]
>
> diff --git a/src/misc/getopt_long.c b/src/misc/getopt_long.c
> [...]
> #include <stdio.h>
> #include <string.h>
> #include "stdio_impl.h"
> +#include "locale_impl.h"
This new include is not needed.
> [...]
> diff --git a/src/misc/getopt_msg.c b/src/misc/getopt_msg.c
> [...]
> +#include <string.h>
> +#include "stdio_impl.h"
> +#include "locale_impl.h"
Same problem like strsignal.c with stddef.h here.
> [...]
>
> From 8ef4199d01c0df6c8622051ba4e4662de6c423d7 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Fri, 18 Sep 2026 14:04:12 -0400
> Subject: [PATCH 15/23] locale: remove now-unused gettext-style translation
> code
>
> [...]
>
> diff --git a/src/internal/locale_impl.h b/src/internal/locale_impl.h
> [...]
> -hidden const char *__lctrans_impl(const char *, const struct __locale_map *);
> [...]
> diff --git a/src/locale/__lctrans.c b/src/locale/__lctrans.c
> [...]
> -static const char *dummy(const char *msg, const struct __locale_map *lm)
> -{
> - return msg;
> -}
> -
> -weak_alias(dummy, __lctrans_impl);
The (previously) non-dummy __lctrans_impl() in locale_map.c still
exists (and is no longer hidden).
> [...]
>
> From fe70f88faf43c60b7f94a3c919eead772a7e5ca9 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Fri, 18 Sep 2026 11:37:06 -0400
> Subject: [PATCH 16/23] add an iterator for unicode normalization form NFD for
> internal use
>
> [...]
>
> diff --git a/src/internal/nfd.h b/src/internal/nfd.h
> [...]
> +#define decomp_iterator_start __decomp_iterator_start
> +#define decomp_iterate __decomp_iterate
> +#define nfd_iterator_start __nfd_iterator_start
> +#define nfd_iterate __nfd_iterate
> +#define nfd_iterate_ccc __nfd_iterate_ccc
Same question as for ikmlt: are these defines really worthwhile?
> +void decomp_iterator_start(struct decomp_iterator *, const void *, int);
> +uint32_t decomp_iterate(struct decomp_iterator *);
> +
> +void nfd_iterator_start(struct nfd_iterator *, const void *, int);
> +uint32_t nfd_iterate_ccc(struct nfd_iterator *);
> +wchar_t nfd_iterate(struct nfd_iterator *);
Shouldn't these functions be hidden?
> [...]
>
> From aba708364983906ee36b1842fb1dc62dd157eefd Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Fri, 18 Sep 2026 12:08:29 -0400
> Subject: [PATCH 17/23] add collation implementation
>
> [...]
>
> diff --git a/src/internal/collate.h b/src/internal/collate.h
> [...]
> +#define ce_iterate __ce_iterate
> +#define ce_iterator_start __ce_iterator_start
> +#define collation_init __collation_init
> +
> +hidden const unsigned char *ce_iterate(struct ce_iterator *);
> +hidden void ce_iterator_start(struct ce_iterator *, const void *, int, const struct collation *);
> +hidden int collation_init(struct collation *, const unsigned char *);
Same question as for ikmlt: are these defines really worthwhile?
> +size_t __strxfrm_core(char *restrict, const void *restrict, size_t, int, const struct collation *);
> +int __strcoll_core(const void *, const void *, int, const struct collation *);
Shouldn't these functions be hidden?
> [...]
> diff --git a/src/locale/locale_map.c b/src/locale/locale_map.c
> [...]
> + collation_init(l->collate, ikmlt_lookup(root, K_COLLATION));
Why does collation_init() return a value if it's unused?
> [...]
> diff --git a/src/locale/strxfrm.c b/src/locale/strxfrm.c
> [...]
> +size_t __strxfrm_core(char *restrict dest, const void *restrict src, size_t n, int wide, const struct collation *col)
> +{
> [...]
> + size_t total_len = nlevels-1;
> +
This empty line has a trailing tab.
> [...]
> diff --git a/src/locale/wcsxfrm.c b/src/locale/wcsxfrm.c
> [...]
> + /* In absence of collation rules, strxfrm behaves as identity. */
This is wcsxfrm(), not strxfrm().
> [...]
>
> From 304767d2576e1c285c76afb71522bd5bbdad85ed Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Thu, 17 Sep 2026 14:11:48 -0400
> Subject: [PATCH 18/23] move global_locale object out of libc struct
>
> [...]
>
> diff --git a/src/internal/libc.h b/src/internal/libc.h
> [...]
> +extern hidden struct __locale_struct __global_locale;
This could be declared in locale_impl.h instead. locale_impl.h would
then no longer need to include libc.h.
> [...]
>
> From c45dbe1d43d5a5aab85f529902f3f57d8149fe7a Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Fri, 18 Sep 2026 11:32:47 -0400
> Subject: [PATCH 19/23] implement non-stub localeconv
>
> [...]
>
> setlocale and newlocale are modified to expand the actual locale data
> into the target locale object. the mutable global locale structure is
> pre-initialized with the C locale values, so that no code is needed to
> prep it, and the static __c_locale and __c_dot_utf8_locale objects
> which exist to make newlocale failure-free for these locales also hold
> copies of this data.
This is no longer the case in v2. Now the lconv pointer is (initially)
a null pointer for these locales and localeconv() returns a pointer to
posix_lconv for them.
> [...]
>
> diff --git a/src/internal/locale_impl.h b/src/internal/locale_impl.h
> [...]
> struct __locale_struct {
> const struct __locale_map *cat[6];
> + struct lconv *lconv;
> };
duplocale() doesn't handle the lconv pointer correctly. It just copies
it, so the two locale objects will share the same lconv structure. If
one of these locale objects is subsequently passed to newlocale(), the
lconv structure will be overwritten for both.
duplocale() could be changed like this:
static void dummy_lconv_expand(locale_t loc)
{
}
weak_alias(dummy_lconv_expand, __lconv_expand);
locale_t __duplocale(locale_t old)
{
locale_t new = malloc(sizeof *new + sizeof *new->lconv);
if (!new) return 0;
if (old == LC_GLOBAL_LOCALE) old = &__global_locale;
*new = *old;
new->lconv = (void *)(new + 1);
__lconv_expand(new);
return new;
}
> [...]
> diff --git a/src/locale/localeconv.c b/src/locale/localeconv.c
> [...]
> +void __lconv_expand(locale_t loc)
> +{
> [...]
> + *lc = posix_lconv;
> +
This empty line has a trailing tab.
> [...]
> + if ((char)-1 > 0) {
> + loc->lconv->grouping += strlen(loc->lconv->grouping) + 1;
> + loc->lconv->mon_grouping += strlen(loc->lconv->mon_grouping) + 1;
> + }
This doesn't work if the value for (mon_)grouping from posix_lconv is
used (e.g. LC_NUMERIC/LC_MONETARY is missing in loc). posix_lconv
could be changed like this:
static const struct lconv posix_lconv = {
// ...
.grouping = "\0",
// ...
.mon_grouping = "\0",
// ...
}
To fix it for all empty strings instead:
if ((char)-1 > 0) {
size_t l = strlen(loc->lconv->grouping);
loc->lconv->grouping += l + !!l;
l = strlen(loc->lconv->mon_grouping);
loc->lconv->mon_grouping += l + !!l;
}
> [...]
> + return;
> +}
The return can be omitted.
> struct lconv *localeconv(void)
> {
> - return (void *)&posix_lconv;
> + /* Prior to first setlocale, or for uselocale(&default_ctype_locale),
> + * the locale's lconv pointer may be null. In either case,
> + * default_lconv accurately reflects the locale. */
Shouldn't this comment say posix_lconv instead of default_lconv?
> [...]
> diff --git a/src/locale/newlocale.c b/src/locale/newlocale.c
> [...]
> + if ((loc = malloc(sizeof *loc + sizeof *loc->lconv))) {
> + *loc = tmp;
> + loc->lconv = (void *)(loc + 1);
Writing it this way would avoid copying a partially uninitialized tmp:
tmp.lconv = (void *)(loc + 1);
*loc = tmp;
> [...]
>
> From 331f8c00a8dba3da34367983fb05569c7e0bc570 Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Sun, 20 Sep 2026 21:16:22 -0400
> Subject: [PATCH 20/23] implement a working, non-stub strfmon
>
> [...]
>
> diff --git a/src/locale/strfmon.c b/src/locale/strfmon.c
> [...]
> static ssize_t vstrfmon_l(char *s, size_t n, locale_t loc, const char *fmt, va_list ap)
> {
> + locale_t old = __uselocale(loc);
> + const struct lconv *lc = localeconv();
> + __uselocale(old);
Other functions that temporarily change the current locale (like
iconv() or fputwc()) just use CURRENT_LOCALE instead of uselocale().
> [...]
>
> From 81ab5823ae00390d6a85dee0fb0a2492403451bb Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Wed, 23 Sep 2026 19:18:57 -0400
> Subject: [PATCH 21/23] add a default locale path and switch to new environment
> variable
>
> [...]
>
> diff --git a/src/locale/locale_map.c b/src/locale/locale_map.c
> [...]
> + if (!path) path = "/etc/musl/locale:/usr/share/musl/locale:/share/musl/locale";
>
> if (path) for (; *path; path=z+!!*z) {
path will now never be a null pointer at this point.
> z = __strchrnul(path, ':');
Two things I noticed for the path search:
A zero-length prefix in the sequence of path prefixes (":", ":/a",
"/a:", "/a::/b") will lead to a lookup in the root directory. execvp()
would search in the current working directory instead.
The condition for detecting prefixes that are too long could be
relaxed:
if (l > sizeof buf - n - 2) continue;
instead of
if (l >= sizeof buf - n - 2) continue;
> [...]
>
> From ef8494a49da47bf0a80c4de27316548a77a7dbeb Mon Sep 17 00:00:00 2001
> From: Rich Felker <dalias@...ifal.cx>
> Date: Wed, 23 Sep 2026 19:34:03 -0400
> Subject: [PATCH 22/23] stop locale path search on any error except
> deterministic unavailability
>
> [...]
>
> unlike with execvp, we don't bother tracking if EACCES was seen
> because setlocale is not specified to report a meaningful error.
But newlocale() is specified to report a meaningful error.
> [...]
[1] https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/basedefs/V1_chap07.html#tag_07_03_05_03
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
