[GitHub] [incubator-kvrocks] jihuayu commented on pull request #1479: Fix all weird out parameters `int *ret` in the codebase

["jihuayu (via GitHub)" <gi...@apache.org>]

jihuayu commented on PR #1479:
URL: https://github.com/apache/incubator-kvrocks/pull/1479#issuecomment-1568371041

   > @jihuayu Please, have a look at `String::CAD` where `flag` is of type `*int` and inside a function, you assign boolean and integer values to the `flag` variable. The first issue is that it's not consistent. The second one is, again, an implicit conversion between an integer value and a boolean value.
   
   I will fix it.
   
   
   > @jihuayu In `CommandSIsMember` you changed `int` to `bool` which is good. However, the line `*output = redis::Integer(ret);` means the boolean value is implicitly converted to an integer value. Am I right?
   
   @torwig Thank you for bringing this to my attention. While it is true that `bool` is a [integral types](https://en.cppreference.com/w/cpp/types/is_integral) that can be passed to the `Integer` function, there is no `std::to_string` function for `bool` parameters. This could potentially lead to implicit type conversion.
   
   There are two possible solutions to the current issue:
   1. Change `*output = redis::Integer(ret)` to `*output = redis::Integer(static_cast<int>(ret))` in order to prevent implicit type conversion.
   2. Add a new `Integer` function to handle cases where the parameter type is `bool`. 
   ```c++
   std::string Integer(bool data) { return Integer(data ? 1 : 0); }
   ```
   
   Personally, I am inclined towards the second solution.  Do you have any other suggestions? 
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org