You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Sterling Hughes <st...@apache.org> on 2016/09/11 17:42:07 UTC

Removing os_error_t

Hey,

Across the OS, we have two interfaces: some that use os_error_t, and the 
other that uses int\u2019s for return codes.

Personally, I have never liked using a typedef for an error code 
(largely as a short hand for an enum.)  I like to have a single variable 
\u201crc\u201d that I use for error accounting \u2014 which works fine with 
enums, but if the type changes under me, then all the code doesn\u2019t 
compile.  Plus, there are cases where I want to return values and error 
codes in a single int (positive = val, negative = error code.)  In order 
to stick with the spirit of an enum, I\u2019d have to separate the value 
and return code, and I\u2019m lazy and dislike functions with unnecessarily 
long signatures.

And \u201cos_error_t\u201d only applies to OS error codes, and our libraries 
are mixed.  FS uses \u2018#defines\u2019, whereas BLE uses enums again.

So \u2014 prior to 1.0, I think we should clean this up.  My proposal is to 
go with plain old integers as error codes across the system.  0 is no 
error, a negative value is the error code and a positive value can be 
used when returning actual data (and must be marked in the function 
signature/doxygen comment.)  Although, if we want to go the enum route, 
I\u2019m happy with that too, but I think we should clean up the code that 
currently uses integers as return values (there is a lot of it), to move 
to cleanly specifying where the error parameters are.

Sterling


Re: Removing os_error_t

Posted by will sanfilippo <wi...@runtime.io>.
Fine with me, but I also do like using BOOLEAN types as well and have functions return either TRUE or FALSE. I think it makes the code easier to read… so I hope we can still use TRUE or FALSE for some functions.

> On Sep 11, 2016, at 11:32 AM, Christopher Collins <cc...@apache.org> wrote:
> 
> On Sun, Sep 11, 2016 at 10:42:07AM -0700, Sterling Hughes wrote:
> [...]
>> So — prior to 1.0, I think we should clean this up.  My proposal is to 
>> go with plain old integers as error codes across the system.  0 is no 
>> error, a negative value is the error code and a positive value can be 
>> used when returning actual data (and must be marked in the function 
>> signature/doxygen comment.)  Although, if we want to go the enum route, 
>> I’m happy with that too, but I think we should clean up the code that 
>> currently uses integers as return values (there is a lot of it), to move 
>> to cleanly specifying where the error parameters are.
> 
> I agree with removing enum return codes, as enums have odd semantics and
> don't provide any real benefit in my opinion [*].  0 for success,
> positive for data, negative for error sounds like a fine convention to
> me.
> 
> Chris
> 
> [*] The type of an enum is implementation defined (4).  The type of an enum
> value is always int (3): 
> 
> (ISO/IEC 9899:201x, 6.7.2.2)
> 3 The identifiers in an enumerator list are declared as constants that
> have type int and may appear wherever such are permitted. An enumerator
> with = defines its enumeration constant as the value of the constant
> expression. If the first enumerator has no =, the value of its
> enumeration constant is 0. Each subsequent enumerator with no = defines
> its enumeration constant as the value of the constant expression
> obtained by adding 1 to the value of the previous enumeration constant.
> (The use of enumerators with = may produce enumeration constants with
> values that duplicate other values in the same enumeration.) The
> enumerators of an enumeration are also known as its members.
> 
> 4 Each enumerated type shall be compatible with char, a signed integer
> type, or an unsigned integer type. The choice of type is
> implementation-defined, but shall be capable of representing the
> values of all the members of the enumeration.The enumerated type is
> incomplete until immediately after the } that terminates the list of
> enumerator declarations, and complete thereafter.


Re: Removing os_error_t

Posted by Christopher Collins <cc...@apache.org>.
On Sun, Sep 11, 2016 at 10:42:07AM -0700, Sterling Hughes wrote:
[...]
> So \u2014 prior to 1.0, I think we should clean this up.  My proposal is to 
> go with plain old integers as error codes across the system.  0 is no 
> error, a negative value is the error code and a positive value can be 
> used when returning actual data (and must be marked in the function 
> signature/doxygen comment.)  Although, if we want to go the enum route, 
> I\u2019m happy with that too, but I think we should clean up the code that 
> currently uses integers as return values (there is a lot of it), to move 
> to cleanly specifying where the error parameters are.

I agree with removing enum return codes, as enums have odd semantics and
don't provide any real benefit in my opinion [*].  0 for success,
positive for data, negative for error sounds like a fine convention to
me.

Chris

[*] The type of an enum is implementation defined (4).  The type of an enum
value is always int (3): 

(ISO/IEC 9899:201x, 6.7.2.2)
3 The identifiers in an enumerator list are declared as constants that
have type int and may appear wherever such are permitted. An enumerator
with = defines its enumeration constant as the value of the constant
expression. If the first enumerator has no =, the value of its
enumeration constant is 0. Each subsequent enumerator with no = defines
its enumeration constant as the value of the constant expression
obtained by adding 1 to the value of the previous enumeration constant.
(The use of enumerators with = may produce enumeration constants with
values that duplicate other values in the same enumeration.) The
enumerators of an enumeration are also known as its members.

4 Each enumerated type shall be compatible with char, a signed integer
type, or an unsigned integer type. The choice of type is
implementation-defined, but shall be capable of representing the
values of all the members of the enumeration.The enumerated type is
incomplete until immediately after the } that terminates the list of
enumerator declarations, and complete thereafter.

Re: Removing os_error_t

Posted by "paul@wrada.com" <pa...@wrada.com>.
+1 for this idea.  I like the idea of a package that holds error codes.

Regarding error codes type, I don¹t have a strong opinion.  My preference
would be enums because my IDE can help me and the compiler can help a bit.

Paul


On 9/11/16, 10:44 AM, "Sterling Hughes" <st...@apache.org> wrote:

>PS: I wonder if we shouldn¹t create a package (sys/error) that
>everything in the system relies upon, that provides standardized error
>codes for the system.
>
>On 11 Sep 2016, at 10:42, Sterling Hughes wrote:
>
>> Hey,
>>
>> Across the OS, we have two interfaces: some that use os_error_t, and
>> the other that uses int¹s for return codes.
>>
>> Personally, I have never liked using a typedef for an error code
>> (largely as a short hand for an enum.)  I like to have a single
>> variable ³rc² that I use for error accounting ‹ which works fine
>> with enums, but if the type changes under me, then all the code
>> doesn¹t compile.  Plus, there are cases where I want to return
>> values and error codes in a single int (positive = val, negative =
>> error code.)  In order to stick with the spirit of an enum, I¹d have
>> to separate the value and return code, and I¹m lazy and dislike
>> functions with unnecessarily long signatures.
>>
>> And ³os_error_t² only applies to OS error codes, and our libraries
>> are mixed.  FS uses Œ#defines¹, whereas BLE uses enums again.
>>
>> So ‹ prior to 1.0, I think we should clean this up.  My proposal is
>> to go with plain old integers as error codes across the system.  0 is
>> no error, a negative value is the error code and a positive value can
>> be used when returning actual data (and must be marked in the function
>> signature/doxygen comment.)  Although, if we want to go the enum
>> route, I¹m happy with that too, but I think we should clean up the
>> code that currently uses integers as return values (there is a lot of
>> it), to move to cleanly specifying where the error parameters are.
>>
>> Sterling


Re: Removing os_error_t

Posted by Sterling Hughes <st...@apache.org>.
PS: I wonder if we shouldn\u2019t create a package (sys/error) that 
everything in the system relies upon, that provides standardized error 
codes for the system.

On 11 Sep 2016, at 10:42, Sterling Hughes wrote:

> Hey,
>
> Across the OS, we have two interfaces: some that use os_error_t, and 
> the other that uses int\u2019s for return codes.
>
> Personally, I have never liked using a typedef for an error code 
> (largely as a short hand for an enum.)  I like to have a single 
> variable \u201crc\u201d that I use for error accounting \u2014 which works fine 
> with enums, but if the type changes under me, then all the code 
> doesn\u2019t compile.  Plus, there are cases where I want to return 
> values and error codes in a single int (positive = val, negative = 
> error code.)  In order to stick with the spirit of an enum, I\u2019d have 
> to separate the value and return code, and I\u2019m lazy and dislike 
> functions with unnecessarily long signatures.
>
> And \u201cos_error_t\u201d only applies to OS error codes, and our libraries 
> are mixed.  FS uses \u2018#defines\u2019, whereas BLE uses enums again.
>
> So \u2014 prior to 1.0, I think we should clean this up.  My proposal is 
> to go with plain old integers as error codes across the system.  0 is 
> no error, a negative value is the error code and a positive value can 
> be used when returning actual data (and must be marked in the function 
> signature/doxygen comment.)  Although, if we want to go the enum 
> route, I\u2019m happy with that too, but I think we should clean up the 
> code that currently uses integers as return values (there is a lot of 
> it), to move to cleanly specifying where the error parameters are.
>
> Sterling