You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Ben Harper <bt...@gmail.com> on 2017/04/10 02:55:20 UTC
Use of os_error_t and OS_OK
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
Re: Use of os_error_t and OS_OK
Posted by Christopher Collins <ch...@runtime.io>.
On Mon, Apr 10, 2017 at 04:42:09PM +0200, Sterling Hughes wrote:
> I don\u2019t 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 \u201cint\u201d and not \u201cos_error_t\u201d 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?
Sounds good to me. I particularly like using 0 rather than SYS_EOK.
It's completely subjective, but I think it makes the code easier to
read.
Chris
Re: Use of os_error_t and OS_OK
Posted by Vipul Rahane <vi...@runtime.io>.
> On Apr 10, 2017, at 7:42 AM, Sterling Hughes <st...@gmail.com> 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?
+1
>
> 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.
>>
>>
>>> On Apr 9, 2017, at 7:55 PM, Ben Harper <bt...@gmail.com> 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
Re: Use of os_error_t and OS_OK
Posted by will sanfilippo <wi...@runtime.io>.
Well, I replied quite differently, but I did not realize that os_error_t was a relic. If that is the case, I agree with what you have here :-)
+1
> On Apr 10, 2017, at 7:42 AM, Sterling Hughes <st...@gmail.com> 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.
>>
>>
>>> On Apr 9, 2017, at 7:55 PM, Ben Harper <bt...@gmail.com> 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
Re: Use of os_error_t and OS_OK
Posted by Sterling Hughes <st...@gmail.com>.
I don\u2019t 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 \u201cint\u201d and not \u201cos_error_t\u201d 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.
>
>
>> On Apr 9, 2017, at 7:55 PM, Ben Harper <bt...@gmail.com>
>> 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
Re: Use of os_error_t and OS_OK
Posted by will sanfilippo <wi...@runtime.io>.
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.
> On Apr 9, 2017, at 7:55 PM, Ben Harper <bt...@gmail.com> 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