From 50d855127f73dff4a6a26ad01f66982c5a927ebb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 11:14:10 +0000 Subject: [PATCH] fix(export): `export X` before X is set keeps the attribute export A A=1 sh -c 'echo "[$A]"' # bash: [1] hellish: [] POSIX: `export name` sets the export attribute, and later assignments are exported. hellish only marked a variable that already existed. For a name not yet set, handle_identifier found no record and stored nothing, so the attribute was lost. `declare -x G` had the same gap in declare_assign. `export -p` and `declare -p B` then said nothing about the name either ("declare: B: not found"). The table can already hold a record with no value, and it already means "declared, not set": ${X+x}, [[ -v ]], set -u, ${!p*} and compgen -v all read a NULL value as unset. env_set keeps the export flag on assignment, and unset drops the record with its attribute. So the fix is to create that record in the two places that should, and to teach the rest to show it the way bash does: - export NAME / declare -x NAME on an unset name: a record with no value, exported; - child environment: skipped, since bash passes nothing, not "X="; - export -p: `export X` without a value, so the list reads back to the same thing (it printed X="" before, which re-created X set); - declare -p X: `declare -x X`; - set / declare with no names: not listed, as in bash; - cd with PWD exported but unset: OLDPWD takes PWD's state, attribute included, where ft_strdup(NULL) would have crashed. tests/scripts/62_export_unset_attribute.sh, graded against bash --posix, covers: - the idiom; - the unset state everywhere (env, ${B+}, set, set -u, the listings); - assignment by =, read, for, printf -v and +=, and declare -x; - unset, export -n and declare +x taking the attribute away; - export -p output fed back to a shell; - cd with an exported unset PWD; - a readonly name exported before it has a value. develop diverges on 9 lines; this branch matches bash byte for byte, with no sanitizer report. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RAmeHfJNm7XjYbMNrQqvkG --- src/builtins/builtin_declare.c | 2 + src/builtins/builtin_set2.c | 18 ++--- src/builtins/collect_and_print_exported.c | 26 +++++-- src/builtins/declare_subscript.c | 8 +- src/builtins/export_helpers2.c | 26 +++++-- src/environment/utils.c | 7 +- src/environment/utils2.c | 3 + tests/scripts/62_export_unset_attribute.sh | 88 ++++++++++++++++++++++ 8 files changed, 148 insertions(+), 30 deletions(-) create mode 100644 tests/scripts/62_export_unset_attribute.sh diff --git a/src/builtins/builtin_declare.c b/src/builtins/builtin_declare.c index a52965bf..f985bc67 100644 --- a/src/builtins/builtin_declare.c +++ b/src/builtins/builtin_declare.c @@ -59,6 +59,8 @@ static int declare_print_one(t_shell *state, const char *name) return (ft_eprintf("%s: declare: %s: not found\n", state->ctx, name), 1); attr_letters(state, e, attrs); + if (!e->value) + return (ft_printf("declare -%s %s\n", attrs, e->key), 0); if (assoc_is(e->value)) fmt = assoc_format(e->value); else if (arr_is(e->value)) diff --git a/src/builtins/builtin_set2.c b/src/builtins/builtin_set2.c index 6bd3559e..f6d72967 100644 --- a/src/builtins/builtin_set2.c +++ b/src/builtins/builtin_set2.c @@ -13,8 +13,9 @@ #include "builtins_private.h" #include "sh_input.h" -/* Print all environment entries in `name=value` form (or just `name` when a - variable has no value). This is the output of bare `set` — it differs from +/* Print all set variables in `name=value` form; a name exported before it + has a value is not set, and bash's `set` leaves it out too. This is the + output of bare `set` — it differs from `env` in that it includes all variables, not just the exported ones, and it does not quote the values. */ /* Arrays list in bash's display form, name=([0]="a" ...), so the @@ -38,17 +39,12 @@ void set_print_env(t_shell *state) while (i < state->env.len) { e = &((t_env *)state->env.ctx)[i]; - if (e->key) + if (e->key && e->value) { - if (e->value) - { - if (arr_is(e->value)) - print_env_array(e); - else - ft_printf("%s=%s\n", e->key, e->value); - } + if (arr_is(e->value)) + print_env_array(e); else - ft_printf("%s\n", e->key); + ft_printf("%s=%s\n", e->key, e->value); } i++; } diff --git a/src/builtins/collect_and_print_exported.c b/src/builtins/collect_and_print_exported.c index de387fff..5240cdb9 100644 --- a/src/builtins/collect_and_print_exported.c +++ b/src/builtins/collect_and_print_exported.c @@ -12,15 +12,29 @@ #include "builtins_private.h" -/* Walk the env table and format each exported entry as `export KEY="value"`. - Variables without a value (exported but never assigned) use an empty - string. All strings are heap-allocated — print_and_free_list owns them. */ +/* One exported entry as `export KEY="value"`. A variable exported but not + yet assigned is listed bare, `export KEY`, as bash lists it: printing + KEY="" would re-create it SET when the list is fed back to the shell. */ +static char *export_line(t_env *e) +{ + char *s; + char *tmp; + + if (!e->value) + return (ft_asprintf("export %s", e->key)); + tmp = dquote_str(e->value); + s = ft_asprintf("export %s=\"%s\"", e->key, tmp); + xfree(tmp); + return (s); +} + +/* Walk the env table and format each exported entry. All strings are + heap-allocated — print_and_free_list owns them. */ static void collect_exported_list(t_shell *st, t_vec *list) { size_t j; t_env *e; char *s; - char *tmp; vec_init(list); list->elem_size = sizeof(char *); @@ -30,9 +44,7 @@ static void collect_exported_list(t_shell *st, t_vec *list) e = &((t_env *)st->env.ctx)[j]; if (e->exported) { - tmp = dquote_str(e->value); - s = ft_asprintf("export %s=\"%s\"", e->key, tmp); - xfree(tmp); + s = export_line(e); vec_push(list, &s); } j++; diff --git a/src/builtins/declare_subscript.c b/src/builtins/declare_subscript.c index ea9becaa..dc679bd3 100644 --- a/src/builtins/declare_subscript.c +++ b/src/builtins/declare_subscript.c @@ -53,8 +53,10 @@ char *declare_assign_eq(const char *word) return (NULL); } -/* One NAME or NAME=VALUE operand: NAME=VALUE assigns (export flag from -x), - a bare NAME with no '=' is a no-op that just accepts the name. +/* One NAME or NAME=VALUE operand: NAME=VALUE assigns (export flag from -x). + A bare NAME with -x marks it exported, or, when it is not set, leaves a + record with no value that holds the attribute until it is, exactly as + `export NAME` does. Without -x a bare NAME just accepts the name. subscript_assign() rewrites key and value in place when the key carries a [subscript], so a plain name falls through it untouched and the two spellings cannot drift apart. */ @@ -68,6 +70,8 @@ void declare_assign(t_shell *state, const char *word, int exprt) { if (exprt && env_get(&state->env, (char *)word)) env_get(&state->env, (char *)word)->exported = true; + else if (exprt && ft_is_valid_ident((char *)word)) + env_set(&state->env, env_create(ft_strdup(word), NULL, true)); return ; } ev = env_create(ft_strndup(word, eq - word), ft_strdup(eq + 1), diff --git a/src/builtins/export_helpers2.c b/src/builtins/export_helpers2.c index 0ed5fda8..a50a03b8 100644 --- a/src/builtins/export_helpers2.c +++ b/src/builtins/export_helpers2.c @@ -97,20 +97,32 @@ static char *export_apply_append(t_shell *st, char *id, char *val) return (joined); } -int handle_identifier(t_shell *st, char *id, char *val, const char *argv0) +/* `export NAME` with no value. An existing variable is marked; a name + that is not set yet gets a record with no value, so the attribute + waits for it: `export X; X=1` exports X (POSIX), and until then X is + still unset -- ${X+set} is empty, no child sees it, and `export -p` + lists it bare, as bash does. */ +static void export_name_only(t_shell *st, char *id) { t_env *e; + e = env_get(&st->env, id); + if (e) + { + e->exported = true; + xfree(id); + return ; + } + env_set(&st->env, env_create(id, NULL, true)); +} + +int handle_identifier(t_shell *st, char *id, char *val, const char *argv0) +{ val = export_apply_append(st, id, val); if (ft_is_valid_ident(id)) { if (!val) - { - e = env_get(&st->env, id); - if (e) - e->exported = true; - xfree(id); - } + export_name_only(st, id); else env_set_shaped(st, (t_env){true, id, val}); return (0); diff --git a/src/environment/utils.c b/src/environment/utils.c index 1ec262dd..53ca94e2 100644 --- a/src/environment/utils.c +++ b/src/environment/utils.c @@ -107,8 +107,9 @@ char *env_to_str(t_env *e) /* Build the NULL-terminated envp[] for execve, including only entries where exported==true. Shell-local variables (exported=false) are - intentionally hidden from child processes. Caller owns the array and - each element. */ + intentionally hidden from child processes, and so is a name exported + before it has a value (`export X` alone): bash passes nothing for it, + not "X=". Caller owns the array and each element. */ char **get_envp(t_shell *state, char *exe_path) { char **ret; @@ -123,7 +124,7 @@ char **get_envp(t_shell *state, char *exe_path) while (++i < state->env.len) { e = &((t_env *)state->env.ctx)[i]; - if (e->exported && !arr_is(e->value)) + if (e->exported && e->value && !arr_is(e->value)) ret[j++] = env_to_str(e); } return (ret); diff --git a/src/environment/utils2.c b/src/environment/utils2.c index fb8f2a50..3578b994 100644 --- a/src/environment/utils2.c +++ b/src/environment/utils2.c @@ -25,6 +25,9 @@ void update_pwd_vars(t_shell *state) pwd = env_get(&state->env, PWD_NAME); if (pwd == NULL) try_unset(state, OLDPWD_NAME); + else if (!pwd->value) + env_set(&state->env, env_create(ft_strdup(OLDPWD_NAME), NULL, + pwd->exported)); else env_set(&state->env, env_create(ft_strdup(OLDPWD_NAME), ft_strdup(pwd->value), pwd->exported)); diff --git a/tests/scripts/62_export_unset_attribute.sh b/tests/scripts/62_export_unset_attribute.sh new file mode 100644 index 00000000..a9108224 --- /dev/null +++ b/tests/scripts/62_export_unset_attribute.sh @@ -0,0 +1,88 @@ +# `export NAME` before NAME has a value. POSIX: the export attribute is +# set, and "subsequent" assignments are exported. Until one comes, NAME +# is still unset: no child sees it, ${NAME+x} is empty, `set` leaves it +# out, and `export -p` / `declare -p` list it without a value. + +# The idiom itself. +export A +A=1 +sh -c 'echo "A in child: [$A]"' + +# Before it is assigned: unset everywhere, listed bare. +export B +echo "B set: [${B+yes}]" +env | grep -c '^B=' || true +export -p | grep -E '^export B($|=)' +declare -p B +set | grep -c '^B=' || true +( set -u; : "$B" ) 2>/dev/null || echo "B unbound under set -u" + +# Any way of assigning it exports it. +export C D E F +read -r C </dev/null || E=from-printf +F+=appended +sh -c 'echo "C=[$C] D=[$D] E=[$E] F=[$F]"' + +# declare -x does the same. +declare -x G +G=2 +sh -c 'echo "G in child: [$G]"' +declare -p G + +# unset takes the attribute away with the variable. +export H +H=3 +unset H +H=4 +sh -c 'echo "H after unset: [${H-unset}]"' + +# export -n and declare +x take it away and leave the name unset. +export I +export -n I +I=5 +sh -c 'echo "I after export -n: [${I-unset}]"' +export J +declare +x J +J=6 +sh -c 'echo "J after declare +x: [${J-unset}]"' + +# export -p output can be fed back to a shell and means the same thing. +export K +L=val +export L +export -p | grep -E '^export (K|L)($|=)' > "${TMPDIR:-/tmp}/exp_$$" +unset K L +. "${TMPDIR:-/tmp}/exp_$$" +rm -f "${TMPDIR:-/tmp}/exp_$$" +echo "K set: [${K+yes}] L=[$L]" +K=k +sh -c 'echo "K=[$K] L=[$L]"' + +# Exporting a name that is already set changes nothing but the attribute. +M=keep +export M +sh -c 'echo "M=[$M]"' + +# cd with PWD exported but unset: OLDPWD takes PWD's state, attribute +# included, and PWD is set again. +( + unset PWD + export PWD + cd / + echo "OLDPWD set: [${OLDPWD+yes}]" + export -p | grep -E '^export OLDPWD($|=)' + echo "PWD=[$PWD]" +) + +# A readonly name exported before it has a value stays readonly. +( + export N + readonly N + declare -p N + N=1 +) 2>/dev/null +echo "readonly N: status $?"