From: Josh Rickmar <joshrickmar@outlook.com>
Subject: Re: Commit e600f1246e15 breaks test_update_conflict_wt_rm_vs_repo_rm on FreeBSD
To: gameoftrees@openbsd.org
Cc: Christian Weisgerber <naddy@mips.inka.de>
Date: Tue, 23 Mar 2021 10:27:26 -0400

On Tue, Mar 23, 2021 at 10:43:01AM +0100, Stefan Sperling wrote:
> 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
> 
> 

ok jrick