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 566f62b186b7ae5c1174418acbf7123ff7150d30 Author: Szymon Janc <[email protected]> AuthorDate: Wed Oct 21 10:36:48 2026 +0200 newt: Check errors when writing files Many generated and saved files were written without checking for errors, so a full disk or a similar I/O failure could leave a truncated file behind while newt reported success: - Close() errors were ignored, mostly in deferred calls, for the dependency, .lst, archive object list and split archive linker script files, for images and manifests, and in CopyFile(). - WriteString() results were ignored when saving pkg.yml, syscfg.yml and target.yml. - pubkey-autogen.c was written through a bufio.Writer whose Flush() result was ignored, and an error from os.Create() was discarded, which would make every following write fail silently. - link_tables.ld.h was never closed and errors were only logged, so the build continued and failed later in the linker. Build the small text files in memory and write them with os.WriteFile(), and check Close() for the files that are streamed. generateLinkTables() now returns an error. Assisted-by: Claude:claude-opus-5-5 --- newt/builder/extcmd.go | 38 ++++++++++---------- newt/builder/selftest.go | 4 ++- newt/builder/targetbuild.go | 16 ++++++--- newt/imgprod/imgprod.go | 12 ++++--- newt/imgprod/v1.go | 16 +++++---- newt/pkg/localpackage.go | 40 ++++++++++----------- newt/target/target.go | 8 ++--- newt/toolchain/compiler.go | 87 ++++++++++++++++----------------------------- util/util.go | 8 +++-- 9 files changed, 107 insertions(+), 122 deletions(-) diff --git a/newt/builder/extcmd.go b/newt/builder/extcmd.go index 4f2eb6eb..ff24b370 100644 --- a/newt/builder/extcmd.go +++ b/newt/builder/extcmd.go @@ -24,6 +24,7 @@ import ( "os" "os/exec" "strconv" + "strings" "github.com/kballard/go-shellquote" log "github.com/sirupsen/logrus" @@ -208,20 +209,14 @@ func getLinkTableEntry(name string, alignment int) string { return entry } -func (t *TargetBuilder) generateLinkTables() { +func (t *TargetBuilder) generateLinkTables() error { var s []string + var linkHeader strings.Builder dir := GeneratedBaseDir(t.target.FullName()) + "/link/include" err := os.MkdirAll(dir, os.ModePerm) if err != nil { - log.Error("Generate link tables error:\n", err) - return - } - - linkHeader, err := os.Create(dir + "/link_tables.ld.h") - if err != nil { - log.Error("Generate link tables error:\n", err) - return + return util.ChildNewtError(err) } // Sort packages so that order of link tables (and so binary layout) @@ -234,20 +229,27 @@ func (t *TargetBuilder) generateLinkTables() { s = append(s, lpkg.LinkTables()...) } - if len(s) == 0 { - return - } + if len(s) > 0 { + settings := t.res.Cfg.SettingValues() + linkTablesAlignment, err := strconv.Atoi(settings.Get("MCU_LINK_TABLES_ALIGNMENT")) + if err != nil { + linkTablesAlignment = LinkTablesAlignmentDflt + } - settings := t.res.Cfg.SettingValues() - linkTablesAlignment, err := strconv.Atoi(settings.Get("MCU_LINK_TABLES_ALIGNMENT")) - if err != nil { - linkTablesAlignment = LinkTablesAlignmentDflt + for _, linkTable := range s { + linkHeader.WriteString(getLinkTableEntry(linkTable, linkTablesAlignment)) + } } - for _, linkTable := range s { - linkHeader.WriteString(getLinkTableEntry(linkTable, linkTablesAlignment)) + // The header is included by the linker script, so it has to exist even + // if there are no link tables. + err = os.WriteFile(dir+"/link_tables.ld.h", []byte(linkHeader.String()), + 0644) + if err != nil { + return util.ChildNewtError(err) } + return nil } //link tables diff --git a/newt/builder/selftest.go b/newt/builder/selftest.go index 97b03651..9f7232a6 100644 --- a/newt/builder/selftest.go +++ b/newt/builder/selftest.go @@ -66,7 +66,9 @@ func (t *TargetBuilder) SelfTestCreateExe() error { "builder in invalid state: missing test package") } - t.generateLinkTables() + if err := t.generateLinkTables(); err != nil { + return err + } if err := t.AppBuilder.Build(); err != nil { return err diff --git a/newt/builder/targetbuild.go b/newt/builder/targetbuild.go index d4eefd8a..3812b183 100644 --- a/newt/builder/targetbuild.go +++ b/newt/builder/targetbuild.go @@ -20,7 +20,7 @@ package builder import ( - "bufio" + "bytes" "fmt" "io/ioutil" "mynewt.apache.org/newt/newt/cfgv" @@ -547,8 +547,7 @@ func (t *TargetBuilder) autogenKeys() error { srcDir := GeneratedSrcDir(t.target.FullName()) - f, _ := os.Create(srcDir + "/pubkey-autogen.c") - w := bufio.NewWriter(f) + w := &bytes.Buffer{} fmt.Fprintln(w, "/* Autogenerated, do not edit. */") fmt.Fprintln(w, "#include <bootutil/sign_key.h>") @@ -570,7 +569,12 @@ func (t *TargetBuilder) autogenKeys() error { fmt.Fprintln(w, " },") fmt.Fprintln(w, "};") fmt.Fprintln(w, "const int bootutil_key_cnt = 1;") - w.Flush() + + if err := os.WriteFile(srcDir+"/pubkey-autogen.c", w.Bytes(), + 0644); err != nil { + + return util.ChildNewtError(err) + } return nil } @@ -600,7 +604,9 @@ func (t *TargetBuilder) Build() error { os.RemoveAll(workDir) }() - t.generateLinkTables() + if err := t.generateLinkTables(); err != nil { + return err + } // Execute the set of pre-build user scripts. if err := t.execPreBuildCmds(workDir); err != nil { diff --git a/newt/imgprod/imgprod.go b/newt/imgprod/imgprod.go index f94524ad..cf8a470e 100644 --- a/newt/imgprod/imgprod.go +++ b/newt/imgprod/imgprod.go @@ -83,7 +83,9 @@ func writeImageFiles(ri image.Image, imgFilename string, hexFilename string, } _, err = ri.Write(imgFile) - imgFile.Close() + if cerr := imgFile.Close(); err == nil && cerr != nil { + err = util.ChildNewtError(cerr) + } if err != nil { return err } @@ -261,9 +263,11 @@ func ProduceManifest(opts manifest.ManifestCreateOpts) error { return util.FmtNewtError("Cannot create manifest file %s: %s", opts.TgtBldr.AppBuilder.ManifestPath(), err.Error()) } - defer file.Close() - - if _, err := m.Write(file); err != nil { + _, err = m.Write(file) + if cerr := file.Close(); err == nil && cerr != nil { + err = util.ChildNewtError(cerr) + } + if err != nil { return err } diff --git a/newt/imgprod/v1.go b/newt/imgprod/v1.go index f3beb8c7..475e511d 100644 --- a/newt/imgprod/v1.go +++ b/newt/imgprod/v1.go @@ -77,9 +77,11 @@ func produceLoaderV1(opts ImageProdOpts) (ProducedImageV1, error) { "Can't open target image %s: %s", opts.LoaderDstFilename, err.Error()) } - defer imgFile.Close() - - if _, err := img.Write(imgFile); err != nil { + _, err = img.Write(imgFile) + if cerr := imgFile.Close(); err == nil && cerr != nil { + err = util.ChildNewtError(cerr) + } + if err != nil { return pi, err } @@ -129,9 +131,11 @@ func produceAppV1(opts ImageProdOpts, return pi, util.FmtNewtError( "Can't open target image %s: %s", opts.AppDstFilename, err.Error()) } - defer imgFile.Close() - - if _, err := img.Write(imgFile); err != nil { + _, err = img.Write(imgFile) + if cerr := imgFile.Close(); err == nil && cerr != nil { + err = util.ChildNewtError(cerr) + } + if err != nil { return pi, err } diff --git a/newt/pkg/localpackage.go b/newt/pkg/localpackage.go index cd32a74b..dae072d7 100644 --- a/newt/pkg/localpackage.go +++ b/newt/pkg/localpackage.go @@ -234,14 +234,10 @@ func (lpkg *LocalPackage) SaveSyscfg() error { return util.NewNewtError(err.Error()) } - file, err := os.Create(lpkg.SyscfgYamlPath()) - if err != nil { + s := lpkg.SyscfgY.YAML() + if err := os.WriteFile(lpkg.SyscfgYamlPath(), []byte(s), 0644); err != nil { return util.NewNewtError(err.Error()) } - defer file.Close() - - s := lpkg.SyscfgY.YAML() - file.WriteString(s) return nil } @@ -255,31 +251,31 @@ func (pkg *LocalPackage) Save() error { return util.NewNewtError(err.Error()) } - file, err := os.Create(pkg.PkgYamlPath()) - if err != nil { - return util.NewNewtError(err.Error()) - } - defer file.Close() + var buf bytes.Buffer // XXX: Just iterate ycfg object's settings rather than calling out // cached settings individually. - file.WriteString("pkg.name: " + yaml.EscapeString(pkg.Name()) + "\n") - file.WriteString("pkg.type: " + + buf.WriteString("pkg.name: " + yaml.EscapeString(pkg.Name()) + "\n") + buf.WriteString("pkg.type: " + yaml.EscapeString(PackageTypeNames[pkg.Type()]) + "\n") - file.WriteString("pkg.description: " + + buf.WriteString("pkg.description: " + yaml.EscapeString(pkg.Desc().Description) + "\n") - file.WriteString("pkg.author: " + + buf.WriteString("pkg.author: " + yaml.EscapeString(pkg.Desc().Author) + "\n") - file.WriteString("pkg.homepage: " + + buf.WriteString("pkg.homepage: " + yaml.EscapeString(pkg.Desc().Homepage) + "\n") - file.WriteString("\n") + buf.WriteString("\n") + + buf.WriteString(pkg.sequenceString("pkg.deps")) + buf.WriteString(pkg.sequenceString("pkg.aflags")) + buf.WriteString(pkg.sequenceString("pkg.cflags")) + buf.WriteString(pkg.sequenceString("pkg.cxxflags")) + buf.WriteString(pkg.sequenceString("pkg.lflags")) - file.WriteString(pkg.sequenceString("pkg.deps")) - file.WriteString(pkg.sequenceString("pkg.aflags")) - file.WriteString(pkg.sequenceString("pkg.cflags")) - file.WriteString(pkg.sequenceString("pkg.cxxflags")) - file.WriteString(pkg.sequenceString("pkg.lflags")) + if err := os.WriteFile(pkg.PkgYamlPath(), buf.Bytes(), 0644); err != nil { + return util.NewNewtError(err.Error()) + } return nil } diff --git a/newt/target/target.go b/newt/target/target.go index 539913b5..03f0b47d 100644 --- a/newt/target/target.go +++ b/newt/target/target.go @@ -291,14 +291,10 @@ func (t *Target) Save() error { return err } - file, err := os.Create(t.TargetYamlPath()) - if err != nil { + s := t.TargetY.YAML() + if err := os.WriteFile(t.TargetYamlPath(), []byte(s), 0644); err != nil { return util.NewNewtError(err.Error()) } - defer file.Close() - - s := t.TargetY.YAML() - file.WriteString(s) if err := t.basePkg.SaveSyscfg(); err != nil { return err diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index ab8aadd0..f920fc05 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -735,24 +735,15 @@ func (c *Compiler) GenDepsForFile(file string, compilerType int) error { return err } - // Write the compiler output to a dependency file. - f, err := os.OpenFile(depPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0666) - if err != nil { - return util.ChildNewtError(err) - } - defer f.Close() + // Append the extra dependencies (.yml files) to the compiler output and + // write it to a dependency file. + objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" + o = append(o, escapeDepsPath(objFile)+": "+c.depsString()...) - if _, err := f.Write(o); err != nil { + if err := os.WriteFile(depPath, o, 0644); err != nil { return util.ChildNewtError(err) } - // Append the extra dependencies (.yml files) to the .d file. - objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" - if _, err := f.WriteString(escapeDepsPath(objFile) + ": " + - c.depsString()); err != nil { - return util.NewNewtError(err.Error()) - } - return nil } @@ -1334,12 +1325,7 @@ func (c *Compiler) generateExtras(elfFilename string, } listFile := elfFilename + ".lst" - f, err := os.OpenFile(listFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, - 0666) - if err != nil { - return util.NewNewtError(err.Error()) - } - defer f.Close() + var lst bytes.Buffer cmd := []string{ c.odPath, @@ -1350,12 +1336,12 @@ func (c *Compiler) generateExtras(elfFilename string, if err != nil { // XXX: gobjdump appears to always crash. Until we get that sorted // out, don't fail the link process if lst generation fails. + if err := os.WriteFile(listFile, nil, 0644); err != nil { + return util.ChildNewtError(err) + } return nil } - - if _, err := f.Write(o); err != nil { - return util.ChildNewtError(err) - } + lst.Write(o) sects := []string{".text", ".rodata", ".data"} for _, sect := range sects { @@ -1368,9 +1354,7 @@ func (c *Compiler) generateExtras(elfFilename string, } o, err := util.ShellCommandLimitDbgOutput(cmd, nil, true, 0) if err == nil { - if _, err := f.Write(o); err != nil { - return util.NewNewtError(err.Error()) - } + lst.Write(o) } } @@ -1382,8 +1366,10 @@ func (c *Compiler) generateExtras(elfFilename string, if err != nil { return err } - if _, err := f.Write(o); err != nil { - return util.NewNewtError(err.Error()) + lst.Write(o) + + if err := os.WriteFile(listFile, lst.Bytes(), 0644); err != nil { + return util.ChildNewtError(err) } } @@ -1538,29 +1524,18 @@ func createSplitArchiveLinkerFile(archiveFile string, /* create a name for this script */ ar_script_name := linkerScriptFileName(archiveFile) - // open the file and write out the script - f, err := os.OpenFile(ar_script_name, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, - 0666) - if err != nil { - return util.NewNewtError(err.Error()) - } - defer f.Close() - - if _, err := f.WriteString("CREATE " + archiveFile + "\n"); err != nil { - return util.NewNewtError(err.Error()) - } - + // write out the script + var script strings.Builder + script.WriteString("CREATE " + archiveFile + "\n") for _, arch := range archFiles { - if _, err := f.WriteString("ADDLIB " + arch + "\n"); err != nil { - return util.NewNewtError(err.Error()) - } + script.WriteString("ADDLIB " + arch + "\n") } + script.WriteString("SAVE\n") + script.WriteString("END\n") - if _, err := f.WriteString("SAVE\n"); err != nil { - return util.NewNewtError(err.Error()) - } + if err := os.WriteFile(ar_script_name, []byte(script.String()), + 0644); err != nil { - if _, err := f.WriteString("END\n"); err != nil { return util.NewNewtError(err.Error()) } @@ -1625,17 +1600,15 @@ func (c *Compiler) CompileArchive(outputFile string, createList bool) error { } if createList { - file, err := os.Create(outputFile) - if err != nil { - return util.NewNewtError(err.Error()) + var list strings.Builder + for _, objFilePath := range objList { + list.WriteString(quoteArg(objFilePath) + "\n") } - defer file.Close() - for _, objFilePath := range objList { - _, err = file.WriteString(quoteArg(objFilePath) + "\n") - if err != nil { - return util.NewNewtError(err.Error()) - } + if err := os.WriteFile(outputFile, []byte(list.String()), + 0644); err != nil { + + return util.NewNewtError(err.Error()) } return nil diff --git a/util/util.go b/util/util.go index 727f349e..33725ce3 100644 --- a/util/util.go +++ b/util/util.go @@ -595,9 +595,11 @@ func CopyFile(srcFile string, dstFile string) error { if err != nil { return ChildNewtError(err) } - defer out.Close() - - if _, err = io.Copy(out, in); err != nil { + _, err = io.Copy(out, in) + if cerr := out.Close(); err == nil { + err = cerr + } + if err != nil { return ChildNewtError(err) }
