New method Client.Extensions to list server extensions

[greatroar]

This allows clients to check for extensions such as posix-rename without having to try an operation that depends on them.

[puellanivis]

It would seem that we could just provide a Extension(name string) (data string, supported bool) rather than needing to allocate and return a map just to then presence-test the one field. The Extension() map[string]string given already runs in O(n) time.

Is there really a use case for needing the whole map of extensions?

[greatroar]

I don't have a direct use case, but I implemented what I felt was the most general and convenient option. I suppose an SFTP debugging tool might require a loop over the extensions. I know it's O(n), but for a typical SFTP server n will be < 10 and just storing the extensions on the Client struct takes O(n) time and space already.

Again, I don't have a use case for the general option, so I can implement Extension (maybe HasExtension?) instead.

[puellanivis]

I think if someone is going to be using this, it’s likely going to be testing for a specific extension, like you have in the tests looking for [email protected] support before making the call.

This sort of thing could easily end up in a hot loop, and so, we should try to make it as efficient as possible for the purpose we’re actually looking to use. Thus a HasExtension(name string) (data string) with a backing map[string]string would probably be much better.

and just storing the extensions on the Client struct takes O(n) time and space already.
Setup code typically can get away with having much slower setup time, because it will only be called rarely. The idea that someone might be testing for extension on a regular basis means having that be O(n) in a hot loop, is one way to get accidentally O(n²) situations.

The setup might very well be O(n) time and space, but each call of the Extensions also itself O(n) time and space, which is just going to be dropped on the floor once we extract the one extension value that we’re interested in. A HasExtension call would however be O(1) time and space, and not cause additional allocations that have to be later garbage collected, and serves a much narrower purpose as well.

If someone were going to use this Extensions call efficiently in a loop, they would want to get the map and then keep it somewhere next to the client, and then do lookups on that, rather than rebuilding every time… or we could just do that code ourselves.

[greatroar]

Ok, replaced by an (inlineable) HasExtension function.

It has two return values, because the draft spec allows the extension data to have zero length. I put the bool first because I considered it the primary return value (matching the Has* name). Do you agree with that, or should the order match that of a map lookup?

[puellanivis]

Great, thanks. 😄