make DSI default timeout dynamically configurable#204
Conversation
Add a dsi_default_timeout field to struct afp_server, initialized to DSI_DEFAULT_TIMEOUT (5s) and overridden per server type after identify: Time Capsule servers get DSI_TIMECAPSULE_DEFAULT_TIMEOUT (30s) to accommodate spinning disk wake-up latency. Both AFP clients now accept a -t/--timeout <seconds> option to let the user override the timeout at launch, taking precedence over the server-type default. The value is threaded through the IPC request structs (afp_server_mount_request, afp_server_connect_request) and applied to the server object after connection. All protocol call sites that previously used the DSI_DEFAULT_TIMEOUT compile-time constant now read server->dsi_default_timeout instead.
|
🤖 Augment PR SummarySummary: This PR makes the DSI request timeout configurable at runtime instead of being a fixed compile-time constant. Changes:
Technical Notes: The per-server default is selected based on server type (Time Capsule vs others), and user overrides take precedence when a positive timeout value is provided. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| unsigned int tx_delay; | ||
|
|
||
| /* DSI request timeout in seconds; set by server type detection or user override */ | ||
| int dsi_default_timeout; |
There was a problem hiding this comment.
Adding dsi_default_timeout to the public struct afp_server changes the struct layout/size, which is an ABI break for any external consumers that access this struct from libafpclient (not just source-level API changes). Can you confirm the project intends this ABI change (e.g., via SONAME bump / documented ABI versioning)?
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| s->server_name_printable); | ||
| s->server_type = AFPFS_SERVER_TYPE_TIMECAPSULE; | ||
|
|
||
| if (s->dsi_default_timeout == DSI_DEFAULT_TIMEOUT) { |
There was a problem hiding this comment.
Using s->dsi_default_timeout == DSI_DEFAULT_TIMEOUT as the “not user overridden” sentinel can’t distinguish between the default 5s and an explicit user choice of 5s, so an explicit 5s set before afp_server_identify() could still be overridden to 30s for Time Capsule.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| break; | ||
|
|
||
| case 't': | ||
| request.dsi_timeout = strtol(optarg, NULL, 10); |
There was a problem hiding this comment.
--timeout is parsed via strtol(optarg, NULL, 10) without checking endptr/errno/range, so invalid or overflow inputs can silently turn into 0/garbage and be ignored or misapplied. This is user-facing and can make it hard to tell whether the override was accepted.
Severity: low
Other Locations
cmdline/cmdline_main.c:516
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| {"recursive", 0, 0, 'r'}, | ||
| {"verbose", 0, 0, 'V'}, | ||
| {"loglevel", 1, 0, 'v'}, | ||
| {"timeout", 1, 0, 't'}, |
There was a problem hiding this comment.
usage() output here still doesn’t mention the new -t/--timeout option, so -h help is now incomplete compared to the manpage and the actual supported flags.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
Add a dsi_default_timeout field to struct afp_server, initialized to DSI_DEFAULT_TIMEOUT (5s) and overridden per server type after identify: Time Capsule servers get DSI_TIMECAPSULE_DEFAULT_TIMEOUT (30s) to accommodate spinning disk wake-up latency.
Both AFP clients now accept a -t/--timeout option to let the user override the timeout at launch, taking precedence over the server-type default. The value is threaded through the IPC request structs (afp_server_mount_request, afp_server_connect_request) and applied to the server object after connection. All protocol call sites that previously used the DSI_DEFAULT_TIMEOUT compile-time constant now read server->dsi_default_timeout instead.