Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes: > BTW, to save a level of indentation, you could handle the "trivial" case > early like this: > > if (!askpass || !*askpass) > return get_pass(prompt); > > and continue without an 'else' branch. That is a good advice in general. Also, when you have a way unbalanced if ... else ... where else clause is very small, it usually is much easier to read if you invert the logic to make if part smaller. > OTOH, it may be worthwhile to set > > pass.use_shell = 1; > > to allow commands that are not just a single plain word. But perhaps this > has security implications - I don't know. How does SSH_ASKPASS gets interpreted by other programs? I think we should follow that example. Other than that, I agree with everything you said in your review. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html