From: Matthias Aßhauer Date: Thu, 23 Apr 2020 18:03:26 GMT Subject: Re: [PATCH v2] ssh: add 'ssh.keyfile' option Message-ID: In-Reply-To: <20200423112110.45405-1-ray@ameretat.dev> >+ switch (variant) { >+ case VARIANT_AUTO: >+ BUG("VARIANT_AUTO passed to push_ssh_options"); Why do we keep replicating this all over push_ssh_options? The wording very much sounds like it's a bug whenever VARIANT_AUTO gets passed to push_ssh_options, no exceptions. In that case we should probably just check for it once at the beginning of push_ssh_options instead of after almost every single if. If there are valid cases where VARIANT_AUTO gets passed to push_ssh_options we should probably clarify the wording as to what parameter combinations are and aren't valid with VARIANT_AUTO. I've included the Sign-Offs and Acks of the commits that introduced the previous copies of this.[1][2] The previous discussion[3] on the mailing list does not seem to mention the duplication. [1] a3f5b66f: "ssh: 'simple' variant does not support -4/-6" 2017-11-20 [2] 3fa5e0d0: "ssh: 'simple' variant does not support --port" 2017-11-20 [3] https://lore.kernel.org/git/20170913215448.84674-1-bmwill@google.com/T/#u Best regards Matthias Aßhauer