From: Stefan Sperling <stsp@stsp.name>
Subject: Re: Commit e600f1246e15 breaks test_update_conflict_wt_rm_vs_repo_rm on FreeBSD
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Tue, 23 Mar 2021 10:43:01 +0100

On Mon, Mar 22, 2021 at 11:07:43PM +0100, Christian Weisgerber wrote:
> Christian Weisgerber:
> 
> > I'm updating the FreeBSD port to got 0.50, and regress/cmdline/update.sh
> > throws an error:
> > 
> > ./update.sh -q -r "/tmp"
> > got: readlink: /tmp/got-test-update_conflict_wt_rm_vs_repo_rm-jQlcrkFR/wt/beta: No such file or directory
> 
> It's an uninitialized variable.
> 
> In worktree.c:delete_blob() we have
> 
>         err = get_file_status(&status, &sb, ie, ondisk_path, -1, NULL, repo);
>         if (err)
>                 goto done;
> 
>         if (S_ISLNK(sb.st_mode) && status != GOT_STATUS_NO_CHANGE) {
>                 char ondisk_target[PATH_MAX];
>                 ssize_t ondisk_len = readlink(ondisk_path, ondisk_target,
>                     sizeof(ondisk_target));
> 
> However, if the open() in get_file_status() fails with ENOENT, then
> there is no *stat() call and sb is never set.  On my FreeBSD machine,
> this leads to S_ISLNK(sb.st_mode) spuriously triggering the readlink()
> branch.

I had seen such failures as well occasionally but could never reproduce
them, and thanks to you I now understand why. Thanks for tracking it down!

I would propose this fix. Callers expect get_file_status() to populate
struct sb in all cases, so it might as well initialize it on entry.

diff 22403ab7111504d4c33872ec85a46d2eaf21a95b /home/stsp/src/got
blob - 3d7fd16f3bc469ada987b984a5a539da754992ca
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -1731,6 +1731,7 @@ get_file_status(unsigned char *status, struct stat *sb
 	unsigned char staged_status = get_staged_status(ie);
 
 	*status = GOT_STATUS_NO_CHANGE;
+	memset(sb, 0, sizeof(*sb));
 
 	/*
 	 * Whenever the caller provides a directory descriptor and a