RFC: Use Go 1.23 iterators for locking+traversing stores.

[mtrmac]

RFC: this is a ~demo, only migrating one function out of several.

This replaces one kind of boilerplate with another, but overall seems a bit less repetitive. The primary benefit is that the code working within the locked stores can return directly, without boilerplate, and there is no need to manually declare a func(…) for the loop body.

The for store, err := range syntax is a bit weird, admittedly, but maybe worth it.

The primary downside is that this really doesn’t work for the exactly-one-closure-call helpers (writeTo…Store):

• Using for store, err := range writeToSingleStore is very unintuitive, but maybe acceptable with good helper names
• But Go doesn’t recognize that the loop body executes exactly once, and that if the loop body always returns, there is no need for the code afterwards to return as well. So every caller would need to add an unreachable invented return there… and that seems a bit much for me.

So, we would end up with two rather different helper patterns instead of ~one. I’m really unsure whether it is worth it. It might well make sense to leave the code as is, and to wait another 1/5/10 years for Go to gain type inference and a convenient closure syntax.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtrmac

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mtrmac]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

I am not an active c/storage contributor but I read the code regularly (i.e. when debugging a podman issue) and this seems to me clearly an improvement over the callback. It feels much more natural to read the callers as loops so I like this a lot.

[mtrmac]

Note to self: This looks almost too easy, without warning callers about the semantics. Maybe range iterateLockingAllLayerStores ?

[mtrmac]

iterateReadLockingAllLayerStores? As usual, I tend towards unwieldy names.