This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit b3e37cf344f9cc19b132b9801f9b221163a7055e Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 17:54:27 2026 +0200 newt/syscfg: Fix nondeterministic chained value references ResolveValueRefs replaced MYNEWT_VAL() references in a single pass over the settings map. With chained references, e.g. A: MYNEWT_VAL(B) and B: MYNEWT_VAL(C), the result depended on map iteration order: A ended up either with the value of C or with the literal "MYNEWT_VAL(C)". Resolve references recursively. References to undefined settings, circular references (including a setting referencing itself) and references to unresolvable settings are reported as unresolved. Previously a self reference was silently left as a literal MYNEWT_VAL() value. Unresolved references now also record the name of the referenced setting, so the error lists it. Assisted-by: Claude:claude-opus-5-5 --- newt/syscfg/syscfg.go | 61 +++++++++++++++++++++++++------ newt/syscfg/syscfg_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 11 deletions(-) diff --git a/newt/syscfg/syscfg.go b/newt/syscfg/syscfg.go index 49da0bb0..337bebf2 100644 --- a/newt/syscfg/syscfg.go +++ b/newt/syscfg/syscfg.go @@ -270,21 +270,60 @@ func (cfg *Cfg) AddPkgNamesSettings(pkgs []*pkg.LocalPackage) { } } +// Replaces values of the form `MYNEWT_VAL(<setting>)` with the value of the +// referenced setting. Chained references are followed. References to +// undefined settings and circular references are recorded as unresolved. func (cfg *Cfg) ResolveValueRefs() { - for k, entry := range cfg.Settings { - refName, val, err := cfg.ExpandRef(strings.TrimSpace(entry.Value)) - if err != nil { - // Referenced setting doesn't exist. Set unresolved setting value - // to 0, this way restrictions can be evaluated and won't create - // spurious warnings. + const ( + refResolving = iota + 1 + refDone + ) + + // setting-name => resolution state + states := map[string]int{} + + // Resolves the specified setting; returns false if the setting contains + // an unresolvable reference. + var resolve func(k string) bool + resolve = func(k string) bool { + switch states[k] { + case refResolving: + // Circular reference. + return false + case refDone: + _, unresolved := cfg.UnresolvedValueRefs[k] + return !unresolved + } + + states[k] = refResolving + defer func() { states[k] = refDone }() + + entry := cfg.Settings[k] + refName := ResolveValueRefName(strings.TrimSpace(entry.Value)) + if refName == "" { + // Not a reference. + return true + } + + entry.ValueRefName = refName + _, ok := cfg.Settings[refName] + resolved := ok && resolve(refName) + if resolved { + entry.Value = cfg.Settings[refName].Value + } else { + // Referenced setting doesn't exist or can't be resolved. Set + // unresolved setting value to 0, this way restrictions can be + // evaluated and won't create spurious warnings. entry.Value = "0" cfg.UnresolvedValueRefs[k] = struct{}{} - cfg.Settings[k] = entry - } else if refName != "" { - entry.ValueRefName = refName - entry.Value = val - cfg.Settings[k] = entry } + cfg.Settings[k] = entry + + return resolved + } + + for k := range cfg.Settings { + resolve(k) } } diff --git a/newt/syscfg/syscfg_test.go b/newt/syscfg/syscfg_test.go new file mode 100644 index 00000000..0e64552d --- /dev/null +++ b/newt/syscfg/syscfg_test.go @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package syscfg + +import ( + "testing" +) + +func valueRefsCfg(vals map[string]string) Cfg { + cfg := NewCfg() + for k, v := range vals { + cfg.Settings[k] = CfgEntry{Name: k, Value: v} + } + + return cfg +} + +// Chained references resolve to the final value regardless of map +// iteration order. +func TestResolveValueRefsChain(t *testing.T) { + // Map iteration order is random; repeat to cover different orders. + for i := 0; i < 50; i++ { + cfg := valueRefsCfg(map[string]string{ + "A": "MYNEWT_VAL(B)", + "B": "MYNEWT_VAL(C)", + "C": "MYNEWT_VAL(D)", + "D": "5", + }) + cfg.ResolveValueRefs() + + for _, k := range []string{"A", "B", "C", "D"} { + if v := cfg.Settings[k].Value; v != "5" { + t.Fatalf("setting %s: got %q, want \"5\"", k, v) + } + } + if cfg.Settings["A"].ValueRefName != "B" { + t.Fatalf("setting A: got ref %q, want \"B\"", + cfg.Settings["A"].ValueRefName) + } + if len(cfg.UnresolvedValueRefs) != 0 { + t.Fatalf("unexpected unresolved refs: %v", cfg.UnresolvedValueRefs) + } + } +} + +// Circular references and references to undefined settings are reported as +// unresolved. +func TestResolveValueRefsUnresolved(t *testing.T) { + for i := 0; i < 50; i++ { + cfg := valueRefsCfg(map[string]string{ + "SELF": "MYNEWT_VAL(SELF)", + "MUT_A": "MYNEWT_VAL(MUT_B)", + "MUT_B": "MYNEWT_VAL(MUT_A)", + "UNDEF": "MYNEWT_VAL(NOT_DEFINED)", + "CHAIN": "MYNEWT_VAL(UNDEF)", + "OK": "1", + }) + cfg.ResolveValueRefs() + + for _, k := range []string{"SELF", "MUT_A", "MUT_B", "UNDEF", + "CHAIN"} { + + if _, ok := cfg.UnresolvedValueRefs[k]; !ok { + t.Fatalf("setting %s not reported as unresolved", k) + } + if v := cfg.Settings[k].Value; v != "0" { + t.Fatalf("setting %s: got %q, want \"0\"", k, v) + } + } + if _, ok := cfg.UnresolvedValueRefs["OK"]; ok { + t.Fatalf("setting OK reported as unresolved") + } + } +}
