search

ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.48k stars 2.52k forks source link

`error.PermissionDenied` vs `error.AccessDenied` #16782

Open squeek502 opened 1 year ago

squeek502 commented 1 year ago

These two error names are common through the Zig codebase, but they seem to be used pretty much interchangeably and don't seem to ever be used to convey unique information, so it seems like one canonical version should be chosen and used everywhere (or a distinction should be made about when to use one over another).

Some evidence for why I think it might make sense to merge them:

In terms of which is more common:

nektro commented 1 year ago

mismatch seems inherited

EPERM 1 Operation not permitted EACCES 13 Permission denied

squeek502 commented 1 year ago

There is no explicit error set that I can find that includes both PermissionDenied and AccessDenied

I was wrong. There's at least one:

https://github.com/ziglang/zig/blob/e078324dbe2b57a4016d91b2d2cca0bfc3ef82aa/lib/std/os.zig#L5591-L5610

ok-ryoko commented 7 months ago

I’m writing Zig bindings to a C library for a Linux system administration application—this is the lens through which I’m viewing this issue.

I want to keep C code and errno out of the main application code, so I’ve defined a single error set type to encompass the errors that the C library functions can return out of band via errno. In my case, errno can be set to EACCES but not EPERM, EPERM but not EACCES, or either EACCES or EPERM depending on the function that was called.

EACCES and EPERM come from the POSIX.1-2001 specification, which has this to say:

[EACCES] Permission denied. An attempt was made to access a file in a way forbidden by its file access permissions.

[EPERM] Operation not permitted. An attempt was made to perform an operation limited to processes with appropriate privileges or to the owner of a file or other resource.

The implication is that EACCES is specific to the permissions on a given file and EPERM indicates a generic deficiency of privilege that may not have anything to do with files, e.g., calling setuid(2) to become an arbitrary user without CAP_SETUID. I could make the argument that a failure to access a file due to insufficient permissions is an instance of an operation that isn’t permitted and that there is therefore no need to distinguish between the two cases.

However, the distinction above turns out to be meaningful for the purpose of troubleshooting my (privileged) application. Thus, I’d like to be able to represent both constants in my error set type. How should I do that? Knowing that, at an upper layer of my application, I will be uniting my library-specific error set type with syscall-specific error set types in std.os, I would rather use the same representations that the standard library does for EACCES and EPERM so as not to end up with redundant errors. Currently, error.AccessDenied and error.PermissionDenied, respectively, serve this role.

If there were only one representation for both EACCES and EPERM in the standard library, then I would want to introduce my own representation for one of EACCES and EPERM to maintain the distinction. However, I may not choose the same representation as the author of another module that is also checking errno. If I should happen to include their module as a dependency, I may find myself writing boilerplate code to unify multiple representations of what is canonically one thing.

Allow me to give a more concrete example where compressing the distinction away can be undesirable: the bpf(2) syscall. Since bpf(2) is a Linux-specific syscall, there’s no corresponding API in std.os—users wanting to leverage an idiomatic Zig interface to std.os.linux.bpf that returns an error union type will need to write their own. They will then encounter the following situation: Not only can bpf(2) set errno to EACCES and EPERM, it uses EACCES to communicate a specific error that has nothing to do with file access permissions:

EACCES For BPF_PROG_LOAD, even though all program instructions are valid, the program has been rejected because it was deemed unsafe. This may be because it may have accessed a disallowed memory region or an uninitialized stack/register or because the function constraints don’t match the actual types or because there was a misaligned memory access. In this case, it is recommended to call bpf() again with log_level = 1 and examine log_buf for the specific reason provided by the verifier.

Meanwhile, EPERM is used in the expected manner—to convey the absence of privileges needed to call bpf(2) in the first place:

EPERM The call was made without sufficient privilege (without the CAP_SYS_ADMIN capability).

I believe that the standard library should equip users with the constructs they need to handle situations such as these should they deem it meaningful for their use cases.

If anything, my suggestion is to rename PermissionDenied to OperationNotPermitted to help dispel conflation with AccessDenied. The current nomenclature of AccessDenied and PermissionDenied confuses me to the same degree as the traditional perror(3) output (“Permission denied” and “Operation not permitted”, respectively).

Nomenclature notwithstanding, I agree that an effort should be made to ensure a consistent mapping from E.ACCES to error.AccessDenied and from E.PERM to error.PermissionDenied across the standard library for the reasons given above.

garrisonhh commented 7 months ago

Thanks for the excellent analysis, all! It seems to me that what this discussion boils down to is that std.os needs to choose whether to consistently respect the errno itself or its perror description in translating errnos to zig errors.

I would argue that using the errno to decide error names would be more intuitive for anyone familiar with developing for *nix, would make porting code to zig easier, and minimize the future role of std.os maintainers. I second error.AccessDenied for EACCES and error.PermissionDenied for EPERM simply because they are already widely used.