On 2022/09/01 11:29:34 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Thu, Sep 01, 2022 at 11:15:55AM +0200, Omar Polo wrote:
> > I see two potential problems here:
> > 
> > > +	if (stat(got_repo_get_path_objects_pack(repo), &sb) == -1) {
> > 
> > 1. possible TOCTOU?  what if the file is changed between this check
> > and its actual use?
> 
> This stats the object/pack *directory* to see if anything in it
> changed since we last listed this directory. If so, we list it again.

Right!  I got confused, sorry, and thanks for correcting me and
providing extra context.  the diff is ok for me.

> We open individual packs in this directory later, and keep them
> open, so there are no TOCTOU issues with our access to those files.
> 
> We keep a cached list of existing pack files to avoid a stat on
> every such file during object access, which is expensive (there can
> be hundreds of pack files in some repositories if people don't keep
> their repos tidy).
> 
> Packs could be deleted and new packs could appear, but modification
> of an existing pack should not happen under normal circumstances.
> And once have the pack file handle open and cached, the file
> won't disappear until we close the handle.
> If a an open pack was modified we would likely run into parsing
> errors in got-read-pack and error out.