DevInputBase::init():reinterpret_cast usage

[iusyk]

There is a code where reinterpret_case is used just to emulate the lvalue semantic. This fix introduces more safe and understandable solution: incapsulate the call of ioctl in the separate function, what allow: 1.Accept just int type. 2.Avoid the usage of reinterpret_cast.

Signed-off-by: Ihor Usyk ihor_usyk@epam.com

[iusyk]

Well, I don't see real value in this change.

It just adds some black magic to plain C function call. It does not enforces type safety, nothing. In a way, this is another, obscure way to write reinterpret_cast. And reinterpret_cast was introduced to cope with C functions in the first place.

If you want to achieve some type safety, you should wrap ioctl(..., EVIOCGRAB, ...) into separate fuction.

I afford to disagree, it exactly enforce the type safety

Well, I don't see real value in this change.

It just adds some black magic to plain C function call. It does not enforces type safety, nothing. In a way, this is another, obscure way to write reinterpret_cast. And reinterpret_cast was introduced to cope with C functions in the first place.

If you want to achieve some type safety, you should wrap ioctl(..., EVIOCGRAB, ...) into separate fuction.

I afford to disagree 1.Using reinterpret_cast is not safe itself, and chance to failany audit (if it will required) 2.reinterpret_cast<void*>(1) is just an attempt to emulate the move semantic. For this purpose we can use real 'move'.

3. Besides, new code really gives the understanding what type is acceptable for the call, otherwise - it is possible to interpret that the following can be used as well reinterpret_cast<void*>(new char[2]['a','b']);

Regarding the function wrapping - yes, I totally agree

[lorc]

2.reinterpret_cast<void*>(1) is just an attempt to emulate the move semantic. For this purpose we can use real 'move'. In this particular case, there is no move semantic, as no objects are moved. You are dealing with dirty ABI tricks there. Actually, such things should be hidden from C++ code, because there is a high change of UB there.

3. Besides, new code really gives the understanding what type is acceptable for the call, otherwise - it is possible to interpret that the following can be used as well reinterpret_cast<void*>(new char[2]['a','b']);

problem with ioctl is that this cast also can be correct. This depends on other parameters of this function. This is OS kernel ABI. Strange things could happen there. Actually, libc hides most of such horrors, but ioctl is very special.

Regarding the function wrapping - yes, I totally agree Yes. That will be a right way

[iusyk]

2.reinterpret_cast<void*>(1) is just an attempt to emulate the move semantic. For this purpose we can use real 'move'. In this particular case, there is no move semantic, as no objects are moved. You are dealing with dirty ABI tricks there. Actually, such things should be hidden from C++ code, because there is a high change of UB there.

1. Besides, new code really gives the understanding what type is acceptable for the call, otherwise - it is possible to interpret that the following can be used as well reinterpret_cast<void*>(new char[2]['a','b']);

problem with ioctl is that this cast also can be correct. This depends on other parameters of this function. This is OS kernel ABI. Strange things could happen there. Actually, libc hides most of such horrors, but ioctl is very special.

Regarding the function wrapping - yes, I totally agree Yes. That will be a right way

fixed

[lorc]

s/intriduces/introduces

There are spelling mistakes in other commit as well. Could you make your editor to spellcheck the commit messages?

Anyways, with the spelling fixed:

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>