On Tue, Mar 05, 2024 at 04:09:07PM -0600, Justin Tobler wrote:
> On 24/03/04 12:10PM, Patrick Steinhardt wrote:
> > We do not report to the caller when rolling back a lockfile fails, which
> > will be needed by the reftable compaction logic in a subsequent commit.
> > It also cannot really report on all errors because the function calls
> > `delete_tempfile()`, which doesn't return an error either.
> >
> > Refactor the code so that both `delete_tempfile()` and
> > `rollback_lock_file()` return an error code.
> >
> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> > ---
> > lockfile.h | 4 ++--
> > tempfile.c | 21 +++++++++++++--------
> > tempfile.h | 2 +-
> > 3 files changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/lockfile.h b/lockfile.h
> > index 90af4e66b2..4ed570d3f7 100644
> > --- a/lockfile.h
> > +++ b/lockfile.h
> > @@ -323,9 +323,9 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path)
> > * for a `lock_file` object that has already been committed or rolled
> > * back.
> > */
> > -static inline void rollback_lock_file(struct lock_file *lk)
> > +static inline int rollback_lock_file(struct lock_file *lk)
> > {
> > - delete_tempfile(&lk->tempfile);
> > + return delete_tempfile(&lk->tempfile);
> > }
>
> question: For a lockfile that is already committed or rolled back, is
> the underlying tempfile still active? I'm trying to figure out if it
> possible for an error to be returned in this scenerio. If not, it might
> be nice to clarify in the comment above that, in addition to being a
> NOOP, no error is returned.
No, it's not. I thought that it being a no-op should be clear enough,
but on the other hand it doesn't hurt either to clarify even further.
I'll refrain from sending a v2 only to add this comment as there haven't
been any other things that need to be addressed yet.
Patrick
Attachment:
signature.asc
Description: PGP signature
