If nothing else, unifying around sys/defs makes sense, although currently -1 in sys/defs is mapped to SYS_ENOMEM and grepping for SYS_EUNKNOWN didn't turn anything up.
I'd still like to work on the patch to get everything in line, although I'd appreciate some guidance on what for a few cases. For more specific errors like OS_BAD_MUTEX and OS_NOT_STARTED, should these be ported into sys/defs as new constants or be remapped to existing codes that may not have the same context? Some of the errors in os_error seem to map well onto different names under sys/defs although the names in os_error seem more descriptive. As an example case there's OS_EINVAL and OS_INVALID_PARM, they seem to have the same meaning and map to sys/defs SYS_EINVAL, but OS_INVALID_PARM is much more widely used, I'd guess for readability reasons, which usage should be carried over? > On Apr 10, 2017, at 7:42 AM, Sterling Hughes > <[email protected]> wrote: > > I don’t think we ever came to agreement, and things are a bit of a mishmash. > Ben brings up a good point. > > Mynewt wide, in my view: > > * os_error is a relic, and sys/defs codes should be used. > > * All functions should return “int” and not “os_error_t” or a specific error > type. > > * 0 and -1 are SYS_EOK and SYS_EUNKNOWN (new value) respectively, and can be > safely returned as numbers not defines. > > * For other errors, the SYS_* equivalents should be returned. > > Thoughts? > > Sterling > >> On 10 Apr 2017, at 16:38, will sanfilippo wrote: >> >> Not sure if anyone answered this. This is just my opinion of course: >> >> * The OS functions should use return type os_error_t. >> * Those functions should return OS_OK or some other OS error. >> * Checks against functions with type os_error_t should be against OS_OK and >> not 0. >> >> The bubbling up of errors, well, not sure. If some function not in the OS >> calls an os function which returns os_error_t does not need to use a return >> type of os_error_t; that can be int. >> w >> >>> On Apr 9, 2017, at 7:55 PM, Ben Harper <[email protected]> wrote: >>> >>> While mucking about in the source I found a few places where the use of >>> OS_OK was either returned and checked against a hardcoded zero, or the >>> other way around, and some function signatures that give os_error_t or int >>> and return the other. The documentation has similar disconnects in portions >>> as to what the return type is, and some functions seem to bubble up the >>> response code from underlying system calls and the type changes as it is >>> returned. I'd like to work through fixing these, but I'm not able to find >>> a single source of truth as to which they should be. Is there currently any >>> set guidance on this? Or would it be fine if I just made my best guesses >>> and brought it together as a PR against the github repository? >>> >>> Thanks for any help you can give on the matter. >>> - Ben Harper
