New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Missing a few LibC Error numbers #9572
Comments
Platform specific LibC error codes defined inside `src/lib_c/*/errno.cr`. Same Error numbers are redefined in `Errno` enum in `src/errno.cr`. - Moved all errno constants from `LibC::*` to `LibC::Errno::*` and removed constants redefinitions from `src/errno.cr` (Thanks @waj for the suggestions) - Added missing error numbers to the following platform specific files - x86_64-darwin - x86_64-dragonfly - x86_64-freebsd - x86_64-openbsd - Sorted all Error codes by the Enum Values Fixes: crystal-lang#9572 Signed-off-by: Aravinda Vishwanathapura <mail@aravindavk.in>
Can we make Errno all the definitions combined from all platforms? Maybe this is what @oprypin was suggesting. Then code is portable because definitions are the same for all platforms. Values might be different, but it doesn't matter because Errno values are never produced by a user, only consumed. Though one will have to use one definition or another in the case of this issue, but that's probably fine. |
The code from https://github.com/crystal-lang/crystal/blob/master/src/file/error.cr#L8-L19 private def self.new_from_errno(message, errno, **opts)
case errno
when Errno::ENOENT
File::NotFoundError.new(message, **opts)
when Errno::EEXIST
File::AlreadyExistsError.new(message, **opts)
when Errno::EACCES
File::AccessDeniedError.new(message, **opts)
else
super message, errno, **opts
end
end Some error numbers will have different meanings based on the context. All errors can't be generalized. {% if flag?(:linux) %}
no_xattr_error = Errno::ENODATA
{% end %}
{% if flag?(:darwin) %}
no_xattr_error = Errno::ENOATTR
{% end %}
private def self.new_from_errno(message, errno, **opts)
case errno
when Errno::ENOENT
File::NotFoundError.new(message, **opts)
when Errno::EEXIST
File::AlreadyExistsError.new(message, **opts)
when Errno::EACCES
File::AccessDeniedError.new(message, **opts)
when no_xattr_error
File::NoXattrDataError.new(message, **opts)
else
super message, errno, **opts
end
end
$ python3 -c 'open("/tmp/nonexisting-dir/file1", "w").write("Hello")'
Traceback (most recent call last):
File "<string>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/nonexisting-dir/file1' |
@aravindavk does this mean you agree with the approach I suggested, or not? |
Sorry if I added more confusion. Your suggestion looks good to hide the internal error codes from the user. But that is a new feature and not related to this issue. I don't think I can spend enough time immediately to analyze all common Exceptions and merging the error codes. I feel the current patch is still useful even if anyone plans to implement the common exceptions(Because to raise new Exception, |
|
@asterite #9572 (comment) is correct. Sent that out as PR #9573 |
I wasn't talking about exceptions, I was just talking about Errno. Basically, if Errno defines A and B in linux, and B and C in Mac, let's define a single Errno that contains all of A, B and C. For Mac the value of A can just be -1 or some other value. It doesn't matter because that value doesn't exist there. |
Makes sense. I can work on moving all the error codes to a single place, defining only error codes which are different in each platform. PR from @oprypin is solving the problem in a different way(with error code duplication). Even with that PR, missing error codes need to be added in multiple files as required and also Let me know if I continue to spend time moving error codes to single place with |
Yes, that's a problem, though this is not the first time we run across a problem of this sort. Occasionally an idea pops up to not show enum values in the docs, but I don't think there is a dedicated issue/discussion for that.
I don't think that's the way to go.
Maybe this just needs to wait on the discussion of #9573. That said, I don't think your PR needs to be blocked on any of these decisions; I commented on #9523 (comment) with one possible solution. |
Below code will not compile (Mac/Darwin returns
ENOATTR
and Linux returnsENODATA
if xattr is not set.ENOATTR
and many others are missing in the Crystal)The text was updated successfully, but these errors were encountered: