updater.go - misuse of os.WriteFile ?

[udf2457]

Given you have:

go-tuf/metadata/updater/updater.go

Line 595 in 830edf8

file, err := os.CreateTemp(update.cfg.LocalMetadataDir, "tuf_tmp")

Its not clear why you are doing this:

go-tuf/metadata/updater/updater.go

Line 601 in 830edf8

err = os.WriteFile(file.Name(), data, 0644)

_, err = file.Write(data) would surely be a far more sensible construct ?

I'm not going to open a PR on it, I'll leave that to your own coding preferences ...

[rdimitrov]

hey, @udf2457, thanks for reaching out.

I agree both can be used with the difference that one allows you to specify the file permissions explicitly so it might be the reason why we chose it.

In any case as you mentioned it's a matter of preference and it doesn't affect performance nor behaviour in any way. The amount of imported packages would be the same too. That said, I think we can close the issue for now unless we see more reasons to change it. Wdyt?

[udf2457]

Huh ? 🤷 Let me spell it out ?

You are already opening file in line 595 with os.CreateTemp.

You have defer file.Close() on the "next" line.

os.WriteFile is the "next" line after that.

Ergo you already have an open file handler.

So ....

If you don't like the default permissions, don't use CreateTemp ? e.g. Maybe use os.MkdirTemp to create a directory in which you can put a file with your preferred permissions ?

Or you could use (f *[File]) Chmod(mode [FileMode]) (https://pkg.go.dev/os#File.Chmod) to change the permissions via the already open file handler ?

So ... just use the already open file handler, no ? Or if you want to keep using WriteFile don't defer the close on the prior line ?

But as presently structured it makes no logical, practical or technical sense ?

[rdimitrov]

Hey, @udf2457, thanks for the additional details. Can you open a PR addressing this per your suggestions? Thank you 🙏

[udf2457]

Thx for merging @kommendorkapten, @rdimitrov ❤ . Will close this.