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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: diff: reduce duplicate code
To:
Tracey Emery <tracey@traceyemery.net>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 1 Apr 2021 20:42:29 +0200

Download raw body.

Thread
On Thu, Apr 01, 2021 at 12:32:29PM -0600, Tracey Emery wrote:
> On Thu, Apr 01, 2021 at 07:50:38PM +0200, Klemens Nanni wrote:
> > Condense error checking to reduce a future diff moving those lines around.
> > ---
> >  got/got.c | 18 +++++-------------
> >  1 file changed, 5 insertions(+), 13 deletions(-)
> > 
> > diff --git a/got/got.c b/got/got.c
> > index 5d1639cc..192b847b 100644
> > --- a/got/got.c
> > +++ b/got/got.c
> > @@ -4381,19 +4381,11 @@ cmd_diff(int argc, char *argv[])
> >  			error = got_worktree_open(&worktree, cwd);
> >  			if (error && error->code != GOT_ERR_NOT_WORKTREE)
> >  				goto done;
> > -			if (worktree) {
> > -				repo_path = strdup(
> > -				    got_worktree_get_repo_path(worktree));
> > -				if (repo_path == NULL) {
> > -					error = got_error_from_errno("strdup");
> > -					goto done;
> > -				}
> > -			} else {
> > -				repo_path = strdup(cwd);
> > -				if (repo_path == NULL) {
> > -					error = got_error_from_errno("strdup");
> > -					goto done;
> > -				}
> > +			repo_path = strdup(worktree ?
> > +			    got_worktree_get_repo_path(worktree) : cwd);
> > +			if (repo_path == NULL) {
> > +				error = got_error_from_errno("strdup");
> > +				goto done;
> >  			}
> >  		}
> >  	} else
> > -- 
> > 2.31.0
> 
> I think that's pretty, but do we want to start using ternary
> conditionals inside of a function? Does it make it harder to audit? Just
> some thoughts ...

It is fine. There's already some code which uses this pattern.