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

From:
Tracey Emery <tracey@traceyemery.net>
Subject:
Re: diff: reduce duplicate code
To:
gameoftrees@openbsd.org
Date:
Thu, 1 Apr 2021 12:46:22 -0600

Download raw body.

Thread
On Thu, Apr 01, 2021 at 08:42:29PM +0200, Stefan Sperling wrote:
> 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.

That's a gateway drug to variable assignments in a conditional, stsp! ;)

Then, that's pretty and ok by me.

-- 

Tracey Emery