From: Christian Schulte <cs@schulte.it>
Subject: Re: Fix for vi editing mode in sftp(1) (PING)
To: Crystal Kolipe <kolipe.c@exoticsilicon.com>, tech@openbsd.org
Date: Wed, 21 Jan 2026 18:50:41 +0100

Am 20.01.2026 um 10:40 schrieb Stuart Henderson:
> On 2026/01/20 04:10, Christian Schulte wrote:
>> Am 20.01.2026 um 00:44 schrieb Crystal Kolipe:
>>>
>>> So what Stuart suggested, whilst technically not absolutely necessary, is good
>>> programming practice.
>>>
>>
>> The manual should be the only source to decide things like this and this
>> only says:
>>
>> el_set() ... Returns 0 on success, -1 on failure.
>> el_get() ... Returns 0 if successful, -1 otherwise.
>>
>> The return value of el_set is not checked either. I did not take a
> 
> If the existing unchecked el_set() calls fail it's not a big problem and
> there isn't really anything sftp(1) can do differently.
> 
> If el_get() fails you can't assume anything about the contents of the
> pointer so it's not safe to use it. (Also remember that this code will
> run with editline implementations other than OpenBSD's so being a bit
> careful with this is appropriate).
> 
> Note the return value of el_get() *is* checked in Complete() in sftp.c.

I agree. You could not write

  (void)el_set(...)

to make it clear the author has verified the outcome of the call does
not have any side effects anywhere. That's why I was referring to the
manual instead of to code of a specific implementation. Ignoring those
return values of el_set() still seems odd to me. Someone searching the
internet may find some tutorial on how to set things up. It may happen
that ssh is linked to a different implementation which just makes all
those el_set() calls return -1 and there is no way to find out about
this without entering a debugger and reading the code. No issue with this.

Regards,
-- 
Christian