On 3/4/21 3:34 PM, Junio C Hamano wrote:
Jeff King <peff@xxxxxxxx> writes:
The feature is definitely useful; I think I did 1eb10f4091 (unix-socket:
handle long socket pathnames, 2012-01-09) in response to a real problem.
Certainly callers could handle the error themselves. The reason I pushed
it down into the socket code was to avoid having to implement in
multiple callers. There are only two, but we'd have needed it in both
sides (credential-cache--daemon as the listener, and credential-cache as
the client).
Ironically, the listening side now does a permanent chdir() to the
socket directory anyway, since 6e61449051 (credential-cache--daemon:
change to the socket dir on startup, 2016-02-23). So we could just do
that first, and then feed the basename to the socket code.
The client side would still need to handle it, though. It could probably
also chdir to the socket directory without any real downside (once
started, I don't think the helper program needs to access the filesystem
at all outside of the socket).
So I dunno. I'd be OK to just rip the feature out in favor of doing
those chdir()s. But that seems like a non-zero amount of work versus
leaving, and the existing code has the benefit that if another caller
shows up, it could benefit from the feature.
I am OK to keep the series as-is, and leave it to a possible future
work to remove the need for chdir even for long paths and not having
to return an error with ENAMETOOLONG; when such an update happens,
the "fail if need to chdir" feature this patch is adding will become
a no-op.
I think I'd like to keep things as I have them now with the "disallow
chdir()" option bit and save the "fork() / bind()" solution for a
later patch series. Simple IPC is large enough as it is and the new
ENAMETOOLONG error will only affect callers who set the bit. A later
patch series can easily test and confirm the "fork() / bind() solution
in isolation and test it on the other Unix hosts and then remove the
bit from those callers (if we want).
Jeff
