CS50 Caesar "handles non-numeric key" Problem

Question

i am new here. I want to make a solution for "CS50 Caesar" problem. Details: https://cs50.harvard.edu/x/2024/psets/2/caesar/

I'm getting these as feedback:
:) caesar.c exists.
:) caesar.c compiles.
:) encrypts "a" as "b" using 1 as key
:) encrypts "barfoo" as "yxocll" using 23 as key
:) encrypts "BARFOO" as "EDUIRR" using 3 as key
:) encrypts "BaRFoo" as "FeVJss" using 4 as key
:) encrypts "barfoo" as "onesbb" using 65 as key
:) encrypts "world, say hello!" as "iadxp, emk tqxxa!" using 12 as key
:) handles lack of argv[1]
:( handles non-numeric key
    timed out while waiting for program to exit
:) handles too many arguments

How can i fix ":( handles non-numeric key timed out while waiting for program to exit" this line?

#include <cs50.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./ceasar key\n");
        return 1;
    }

    if (atoi(argv[1]) == 0)
    {
        printf("Usage: ./ceasar key\n");
        return 1;
    }

    int k = atoi(argv[1]);
    k = k % 26;
    string plaintext = get_string("plaintext:  ");
    string ciphertext = plaintext;
    int n = strlen(plaintext);

    for (int i = 0; i < n; i++)
    {
        if ('a' <= plaintext[i] && plaintext[i] <= 'z')
        {
            if ((plaintext[i] + k) > 'z')
            {
                ciphertext[i] = ((plaintext[i] + k) - 26);
            }
            else
            {
                ciphertext[i] = (plaintext[i] + k);
            }
        }
        else if ('A' <= plaintext[i] && plaintext[i] <= 'Z')
        {
            if ((plaintext[i] + k) > 'Z')
            {
                ciphertext[i] = ((plaintext[i] + k) - 26);
            }
            else
            {
                ciphertext[i] = (plaintext[i] + k);
            }
        }
    }
    printf("ciphertext: %s\n", ciphertext);
    return 0;
}

How can i fix ":( handles non-numeric key timed out while waiting for program to exit" this line?

Answer 1

atoi() can return non-zero, when the argument is not completely numeric.
atoi("123x") returns 123.
atoi("x") is a conversion error and the result is undefined behavior (UB) - it might not return 0. Testing atoi() result for 0 is an insufficient non-numeric test.

Use strtol() to validate and convert argv[1] instead of atoi().

// if (atoi(argv[1]) == 0) {
//     printf("Usage: ./ceasar key\n");
//     return 1;
// }
// int k = atoi(argv[1]);

errno = 0;
char *endptr;
long lvlaue = strtol(argv[1], &endptr, 0);
if (argv[1] == endptr) {
  printf("arg1 is not numeric\n"); return 1;
}
if (*endptr != 0) {
  printf("arg1 is not completely numeric\n"); return 1;
}
if (errno || lvalue < INT_MIN || lvlaue > INT_MAX) {
  printf("arg1 is too large\n"); return 1;
}
int k = (int) lvalue;

Code could wrap this into one big if and use a long k.

errno = 0;
char *endptr;
long k = strtol(argv[1], &endptr, 0);
if ((argv[1] == endptr) || (*endptr != 0) || errno) {
  printf("Bad arg1\n");
  return 1;
}

---

Code also does not perform correctly if the k < 0. Handle negative k as below.

// k = k % 26;

k = k % 26;
if (k < 0) k += 26;