Add SSHMaster::Connection::trySetBufferSize

[Ericson2314]

Motivation

It is unused in Nix currently, but will be used in Hydra. This reflects what Hydra does in NixOS/hydra#1387.

Context

We may probably to use it more widely for better SSH store performance, but this needs to be subject to more testing before we do that.

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[Ericson2314]

Oh wat, segfault in CI in ambient Nix (i.e. nothing to do with this PR)

/Users/runner/work/_temp/8e2e66c1-0beb-427b-912b-9e63f8be4be1.sh: line 1:  4963 Segmentation fault: 11  nix --experimental-features 'nix-command flakes' flake check -L

[roberth]

segfault in CI in ambient Nix (i.e. nothing to do with this PR)

/Users/runner/work/_temp/8e2e66c1-0beb-427b-912b-9e63f8be4be1.sh: line 1:  4963 Segmentation fault: 11  nix --experimental-features 'nix-command flakes' flake check -L

Is that on Darwin? Can you get a stack trace for it?

[vcunat]

We didn't post here, but the fcntl calls are wrong. Setter takes an int, not a pointer. And getter takes nothing, returns value. EDIT: that's how man fcntl.2 reads to me, at least.

[vcunat]

Nit: why design the interface with size_t when we're only able to implement int?

EDIT: especially note that the (current) implementation silently overflows to non-sensical values.

[Ericson2314]

@vcunat ah I could assert, or that + change type to unsigned int. I just didn't want to accept negative numbers.

[roberth]

Could you add a test? We can't just rely on types here, unfortunately.
You could detect the buffer size by filling a pipe in non blocking mode, without reading it.

[vcunat]

Accepting negative numbers? Well, if you do an unsigned int or size_t and pass a too large value, you can end up passing a negative number to the syscall. So if you don't have code somehow guarding that, I'd say it's better to make the real type visible in the API already.

But perhaps more generally: the function returns the resulting buffer size (or negative error code), so perhaps it would make sense to check the return value? Overriding buffer size surely won't be a very important error, but perhaps some debug-level logs in case the value differs from what was passed as desired size? (that could even cover these negative cases by itself)

EDIT: I don't know the customs of Nix codebase, and I only really use pure C, so... take my comments with a grain of salt.