"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
James Cook <falsifian@falsifian.org>
Subject:
Re: got-portable: Fix "could not parse reference data" on NetBSD
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 04 Apr 2025 03:22:02 +0000

Download raw body.

Thread
> > The below patch explicitly checks whether the ref is a directory.
> > Maybe this should go in non-portable got too, at least to keep
> > the codebases more similar.
> 
> Could you change the S_ISDIR check to !S_ISREG? That would also cover
> device nodes, sockets, fifos, etc.
> ok stsp@ with that change, provided it still works as expected for
> you and the tests are passing.

Done. Should I push the same patch to got-portable too? I'm not
sure what the process is.

(got-portable regress is already broken on NetBSD, so I haven't
tried that, but I'm using got-portable with the change.)

> > (By the way, I noticed that if fstat fails, parse_ref_file doesn't
> > call get_lockfile_unlock in the cleanup path. Is that an oversight,
> > or is it that way because we know the program's going to stop soon
> > anyway, or is it something else? Every other "goto done;" unlocks,
> > so if it's okay to unlock in that case too, the unlock could be
> > moved to "done:" to make the code a bit tidier.)
> 
> Looks like an oversight to me. Could you write a fix for this, too?
> 
> Thanks!

See patch below.

Moving it to "done:" is more complicated than I thought, because
sometimes the lock is kept when there's no error.

-- 
James

diff /home/falsifian/co/got
path + /home/falsifian/co/got
commit - e896de0dbd9971fbc7969f0df170b4a495b4cd28
blob - 90462626d8b4280e7a38aa3e03f64545013c53f6
file + lib/reference.c
--- lib/reference.c
+++ lib/reference.c
@@ -206,6 +206,8 @@ parse_ref_file(struct got_reference **ref, const char 
 	}
 	if (fstat(fileno(f), &sb) == -1) {
 		err = got_error_from_errno2("fstat", abspath);
+		if (lock)
+			got_lockfile_unlock(lf, -1);
 		goto done;
 	}
 	if (!S_ISREG(sb.st_mode)) {