ccharfgetsstrtol

Narrowing conversion from 'long' to signed type 'char' is implementation-defined (strtol function in C)


I'm trying to convert an inputted character to an integer by using strtol. Here's part of the source code:

char option;
char *endptr;

printf("=========================================Login or Create Account=========================================\n\n");
while(1) {
    printf("Welcome to the Bank management program! Would you like to 1. Create Account or 2. Login?\n>>> ");
    fgets(&option, 1, stdin);
    cleanStdinBuffer();
    option = strtol(&option, &endptr, 10);

In the strtol function, I'm getting a warning saying:

Clang-Tidy: Narrowing conversion from 'long' to signed type 'char' is implementation-defined

Can someone tell me what I'm doing wrong?


Solution

  • Clang-Tidy is warning you about the implicit conversion you are doing here where you are assign the long return value of strtol to a char:

    option = strtol(&option, &endptr, 10);
    

    If this is intentional and you are sure the value will be in the [-128,127] range that isn't necessarily an issue (it's just a warning), but even then I would advice to explicitly cast the return-type of strtol, use int8_t instead of char and not reuse the option variable for the return value. In other words:

    int8_t value = (int8_t)strtol(&option, &endptr, 10);
    

    If it wasn't intentional I would recommend you to simply use long as type for the variable you assign the return value of strtol, so:

    long value = strtol(&option, &endptr, 10);
    

    What Clang-tidy doesn't warn you about is that the first argument to strtol should be a pointer to a char buffer containing a 0-terminated string, not a pointer to a single char. This is also an issue for fgets. There are two ways to solve this, either:

    1. Make option a char array of at least two chars,

    2. Use fgetc instead and modify your code into something like this:

      int option = fgetc(stdin);
      if (option == '1') {
          /*Create Account */
      } else if (option == '2') {
          /* Login */
      }
      else {
          /* Error */
      }
      

    I think the latter looks much cleaner.