[libc] Add printf error handling

[michaelrj-google]

Currently printf never sets errno, which is incorrect. The file variants should propogate errors from the file they're writing to and every variant should set EOVERFLOW if the return value would overflow an int. Currently the Writer class stores the number of chars written as an int, so it should be changed to something larger and probably unsigned. I'd recommend either size_t or uint64_t.

It's important to remember that errno should only be set in public functions before returning, not in internal helpers. The return value of printf_main might need to be changed to be a struct with size_t char count and an error code.

[llvmbot]

@llvm/issue-subscribers-libc

Author: Michael Jones (michaelrj-google)

Currently printf never sets errno, which is incorrect. The file variants should propogate errors from the file they're writing to and every variant should set `EOVERFLOW` if the return value would overflow an `int`. Currently the `Writer` class stores the number of chars written as an `int`, so it should be changed to something larger and probably unsigned. I'd recommend either `size_t` or `uint64_t`.

It's important to remember that errno should only be set in public functions before returning, not in internal helpers. The return value of printf_main might need to be changed to be a struct with size_t char count and an error code.

[mleleszi]

Hey @michaelrj-google, I'd like to work on this

[mleleszi]

@michaelrj-google I've looked at the current implementation, and as far as I can see currently printf returns predefined error values, like FILE_WRITE_ERROR, FIXED_POINT_CONVERSION_ERROR etc defined in printf_core on errors. This issue is about adding error handling for IO errors and adding overflow encoding error for the return value, but printf can fail for other reasons as well like conversion error, allocation errors etc. Wouldn't it make sense to standardize all error handling for printf and its variants?

What I'm thinking is to not have printf return these predefined error values, but always return -1, and set errno based on the actual IO error propagated and some mapping from the current return values to errno for the other errors.
Is this something we can change? As far as i know the standard doesn't mandate specific return values on error other than it being negative, but not sure if changing this would break any existing users who depend on the specific return values.

This would also make the implementation a lot easier, otherwise we'd have some errors reported by errno and some by return values, and that would make propagating the errors harder from the internals.

[michaelrj-google]

Remapping from internal error numbers to errno values seems like a good strategy. We can absolutely change the error values that printf returns. One thing to remember is that we try to set errno only in the public function so you might need to add something like this to each printf entrypoint:

if(error_value != 0) {
    libc_errno = errno_mapping(error_value);
    return -1;
}