No more sys_errlist #8234
No more sys_errlist #8234
Conversation
This no longer works on new glibcs because they removed sys_errlist. So just hardcode the relevant errno messages (and phrase them better). Fixes fish-shell#4183.
This also calls safe_strerror, which isn't usable anymore on new glibc. It is also ridiculous since the errors are bad to begin with. We only use it for setpgid and fork, and I doubt there's gonna be a lot more added to it.
| break; | ||
| } | ||
| case ETXTBSY: { | ||
| FLOGF_SAFE(exec, "Failed to execute process '%s': File is currently open for writing.", actual_cmd); |
There was a problem hiding this comment.
I specifically made these messages not just the ones we get by default.
That's because "text file is busy" is among the worst error messages I have ever read, mostly because it has a 98% chance of appearing when the file is not a text file (in the sense any user cares about).
It's kinda like if an ice cream place told you "machine not fillable" when they're out of a certain flavor. Might be true in the technical sense, but so misleading that it might as well not be said, at all.
There was a problem hiding this comment.
So I would welcome more wording suggestions.
| @@ -415,10 +456,31 @@ void safe_report_exec_error(int err, const char *actual_cmd, const char *const * | |||
| FLOGF_SAFE(exec, "Out of memory"); | |||
| break; | |||
| } | |||
|
|
|||
| case EACCES: { | |||
| FLOGF_SAFE(exec, "Failed to execute process '%s': The file could not be accessed.", actual_cmd); | |||
There was a problem hiding this comment.
probably run clang-format even if it makes this look slightly worse
Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
|
We can use |
I don't know if strerrordesc_np provides any better guarantees than |
Now unused too
Now unused
Uh, yeah, EISDIR means it *is* a directory
|
Okay, tested on arch, alpine, FreeBSD, NetBSD, and Ubuntu and macOS on Github Actions. It seems to compile everywhere and I can't find somewhere the errors are actually wrong, so I'm merging this now because it improves the status quo and I'd like to have it in 3.4.0. More improvement is of course welcome! |
|
I mean, you can’t beat error messages custom tailored to the faulting call - great work @faho. |
|
|
Description
This removes safe_strerror and safe_perror (which uses the former internally), and replaces it with hardcoded error messages.
The impetus for this is that glibc actually removed sys_errlist and according to #7006 we can't use strerror_r either.
I looked up the used errno values I could find. It's possible these are incomplete, in which case we get more "Unknown error $errno" messages, but I don't think that should be a common thing, and on current glibc that's already all of the errors here (except for the exec errors that we already explained).
Fixes issue #4183.
Ideally I would like this to be included in 3.4.0, provided we can get enough testing.
TODOs: