On 02/18/2016 06:42 AM, Jesus Sanchez-Palencia wrote:
Hi,
On Thu, 18 Feb 2016 09:11:59 -0200 Andre Guedes <andre.guedes(a)intel.com> wrote:
Hi Daniel,
Quoting Kalowsky, Daniel (2016-02-17 05:09:07)
DEV_NO_SUPPORT seems to cover the concept. Not sure we need a
DEV_NOT_IMPLEMENTED. Or as Peter points out ENOSYS works as well.
Yes, DEV_NO_SUPPORT might cover the concept however it brings another connotation which is "the hardware actually lacks support" (e.g. the controller doesn't support that feature) and for that case I think the error code DEV_INVALID_OP might be more suitable.
Good point. You're alternate point (in another post) of ENOSYS not matching is also valid.
I think ENOSYS would perfectly match what we want here since it means "Function not implemented" (as Peter pointed in the other reply).
Speaking of which, I was talking to Dirk in the other day and we both agreed that it would be a good idea we use errno.h codes instead of DEV_* at the driver's layer. The main points are 1) errno.h is a well-known error convention which pretty much all developer is familiar with, 2) errno.h codes address what we need, 3) no need to create new codes such as DEV_NOT_IMPLEMENTED for instance, and 4) changing the current drivers to use errno.h codes instead of DEV_* is a feasible task.
A good first step is to redefine DEV_* in terms of -E* error codes :-)
Adding the include of errno.h and the removal of DEV_* can happen at our leisure :-) The apps will be getting a consistent set of error codes in any case.
What do you think?
Just so we avoid mixing two different topics, it looks like this deserves a separate RFC.
The original problem I raised on that code review a few weeks ago -- not having a defined return code to state that a given driver API impl is dummy -- seems to have had some conclusion reached here already. Looks like adding DEV_NOT_IMPLEMENTED has some appeal in the end. I also support that meanwhile the errno idea is not discussed.
I think you've convinced me. The challenge now is to make sure it's used appropriately. As we've shown, we'll need to be explicit with why some functions actually need to return DEV_OK or whatever vs DEV_NO_SUPPORT.
Yes, this is the challenge. I volunteer to review and fix what we have upstream. However, we have to pay attention for this kind of things while reviewing new patches on Gerrit to ensure they are merged with the proper return codes.
Agreed
Guidelines, folks. Let's make sure this is documented somewhere so we can just copy&paste a link on reviews afterwards and people can refer to it when coding.
Great idea! This will help with readability of driver code and if the app developer actually tries to decode the error they will know where to get the *meaning*
