Skip to content

Commit

Permalink
init: set locale after importing full environment (re: c692ee1)
Browse files Browse the repository at this point in the history
The previous commit introduced a mysterious regression test failure
on the GitHub CI runner (and nowhere else that I could find) while
running the regression tests in the nl_NL.UTF-8 locale:

test libcmd(nl_NL.UTF-8) begins at 2024-12-26+07:51:49
	libcmd.sh[856]: FAIL: PATH_LEADING_SLASHES handling
	(expected $'/\n//\n//\n//\n//\n//server\n//server', got
	$'/\n/\n/\n/\n/\n/server\n/server')
test libcmd(nl_NL.UTF-8) failed at 2024-12-26+07:51:50 with exit
code 1 [ 142 tests 1 error ]

Analysis: It seems impossible for the problem to be in the leading
slashes handling code, so the only possibility left is that the
_AST_FEATURES export (to cause 'getconf PATH_LEADING_SLASHES' to
return 1) in libcmd.sh:854 did not take effect.

_AST_FEATURES is read once, on the first call to astconf and
related functions. See the INITIALIZE macro in astconf.c.

After many hours of experimenting I found that, in the OS/config/
locale combination on the GitHub CI runner, importing locale
variables from the environment via env_init() in init.c may, very
indirectly, trigger a call to astconf before the environment is
fully imported. This will then potentially check for _AST_FEATURES
before it is imported, and it won't check again after that.

The relevant part of the call stack for this situation is:

  12 astconf -- called in pathbin because getenv("PATH") returned
     NULL, indicating environment is not fully imported, thus also
     causing _AST_FEATURES to fail to be honoured
  11 pathbin (pathbin.c:37)
  10 pathpath_20100601 (pathpath.c:128)
  09 pathpath (pathpath.c:40)
  08 mcfind (mc.c:185)
  07 single (setlocale.c:2394)
  06 _ast_setlocale (setlocale.c:480; macro at ast_std.h:138)
  05 put_lang (init.c:480)
  04 nv_putv (nvdisc.c:144)
  03 nv_putval (name.c:1645)
  02 nv_open (name.c:1511)
  01 env_init (init.c:1913)

So, long story short, since setting the locale via _ast_setlocale
may require a fully imported environment, we must implement a way
to stop the put_lang discipline function from calling setlocale
while env_init is importing variables like LANG, LC_ALL, etc, and
defer these calls until after the environment is fully imported.

src/cmd/ksh93/include/shell.h:
- We can use a new state flag for this. The SH_PREINIT state bit is
  unused as of 921bbca. Replace it with a new SH_LCINIT state bit.

src/cmd/ksh93/sh/init.c:
- put_lang: While the SH_INIT flag is on (we're initialising the
  shell) but not SH_LCINIT, defer the put_lang action by storing
  the parameters of the put_lang calls in a linked list.
- env_init: After completing the loop that imports all the env
  vars, walk through the created linked list and execute all the
  deferred put_lang calls with SH_LCINIT set. Now, _ast_setlocale
  will be called with the full environment available.
  • Loading branch information
McDutchie committed Dec 28, 2024
1 parent c692ee1 commit 4a24320
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ typedef union Shnode_u Shnode_t;
#define SH_INIT 15 /* set when initializing the shell */
#define SH_TTYWAIT 16 /* waiting for keyboard input */
#define SH_FCOMPLETE 17 /* set for filename completion */
#define SH_PREINIT 18 /* set with SH_INIT before parsing options */
#define SH_LCINIT 18 /* set with SH_INIT while initializing locale */
#define SH_COMPLETE 19 /* set for command completion */
#define SH_XARG 21 /* set while in xarg (command -x) mode */
#define SH_NOTILDEXP 22 /* set to disable tilde expansion */
Expand Down
41 changes: 36 additions & 5 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -424,11 +424,33 @@ static void put_cdpath(Namval_t* np,const char *val,int flags,Namfun_t *fp)
sh.cdpathlist = path_addpath((Pathcomp_t*)sh.cdpathlist,val,PATH_CDPATH);
}

static struct put_lang_defer_s
{
Namval_t *np;
const char *val;
int flags;
Namfun_t *fp;
struct put_lang_defer_s *next;
} *put_lang_defer;

/* Trap for the LC_* and LANG variables */
static void put_lang(Namval_t* np,const char *val,int flags,Namfun_t *fp)
{
int type;
char *name = nv_name(np);
char *name;
if (val && sh_isstate(SH_INIT) && !sh_isstate(SH_LCINIT))
{
/* defer setting locale while importing environment */
struct put_lang_defer_s *p = sh_malloc(sizeof(struct put_lang_defer_s));
p->np = np;
p->val = val;
p->flags = flags;
p->fp = fp;
p->next = put_lang_defer;
put_lang_defer = p;
return;
}
name = nv_name(np);
if(name==(LCALLNOD)->nvname)
type = LC_ALL;
else if(name==(LCTYPENOD)->nvname)
Expand All @@ -454,13 +476,9 @@ static void put_lang(Namval_t* np,const char *val,int flags,Namfun_t *fp)
if(type>=0 || type==LC_ALL || type==LC_NUMERIC || type==LC_LANG)
{
char* r;
#ifdef AST_LC_setenv
ast.locale.set |= AST_LC_setenv;
#endif
r = setlocale(type,val?val:"");
#ifdef AST_LC_setenv
ast.locale.set ^= AST_LC_setenv;
#endif
if(!r && val)
{
if(!sh_isstate(SH_INIT) || !sh_isoption(SH_LOGIN_SHELL))
Expand Down Expand Up @@ -1913,6 +1931,19 @@ static void env_init(void)
}
if((cp = nv_getval(SHELLNOD)) && (sh_type(cp)&SH_TYPE_RESTRICTED))
sh_onoption(SH_RESTRICTED); /* restricted shell */
/*
* Since AST setlocale() may use the environment (PATH, _AST_FEATURES),
* defer setting locale until all of the environment has been imported.
*/
sh_onstate(SH_LCINIT);
while (put_lang_defer)
{
struct put_lang_defer_s *p = put_lang_defer;
put_lang(p->np, p->val, p->flags, p->fp);
put_lang_defer = p->next;
free(p);
}
sh_offstate(SH_LCINIT);
}

/*
Expand Down

0 comments on commit 4a24320

Please sign in to comment.