Re: [PATCH v2 1/3] factor out refresh_and_write_cache function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 29 Aug 2019 at 20:28, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> +int repo_refresh_and_write_index(struct  repository *repo,
> +                                unsigned int refresh_flags,
> +                                unsigned int write_flags,
> +                                const struct pathspec *pathspec,
> +                                char *seen, const char *header_msg)
> +{
> +       struct lock_file lock_file = LOCK_INIT;
> +
> +       repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR);
> +       if (refresh_index(repo->index, refresh_flags, pathspec, seen, header_msg))
> +               return 1;
> +       if (write_locked_index(repo->index, &lock_file, COMMIT_LOCK | write_flags))
> +               return -1;
> +       return 0;
> +}

AFAIU, the_repository->index == &the_index so this patch is a noop on
the converted user as far as that aspect is concerned.

There's a difference in behavior that I'm not sure about: We used
to ignore the return value of `refresh_cache()`, i.e. we didn't care
whether it had any errors. I have no idea whether that's safe to do --
especially as we go on to write the index. So I don't know whether this
patch fixes a bug by introducing the early return. Or if it *introduces*
a bug by bailing too aggressively. Do you know more?

(This conversion provides REFRESH_QUIET, which seems to suppress certain
errors, but not all.)

In any case, that early return introduces a bug with the lockfile, that
much I know. We need to roll back the lockfile before doing the early
return. I should have seen that already in your previous version.. :-(

The above makes me think that once this new function is in good shape,
the commit introducing it could sell it as "this is hard to get right --
let's implement it correctly once and for all". ;-)

Martin



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux