ccs50luhn

there is a problem with my luhn checksum in cs50 that I can't figure out


this the cs50 problem in question : credit

the logic in my checksum seem to be ill constructed as it doesn't recognize 4222222222222 as valid even though it sums is 40 and falsely indicates 4111111111111113 as visa even though it sums is 28 other than these two it labels any other credit card correctly

this is my code :

#include <cs50.h>
#include <ctype.h>
#include <math.h>
#include <stdio.h>

bool digit(string str, int buffer);
int numofdigits(long long card_num);
int fst_ndigit(long long credit, int buffer, int numdigit);

int main(void)
{

    long long credit = get_long("please enter your credit card number :  ");

    //Note:
    //using a boolean variable can reduce repetition of " INVALID " message
    bool valid = true;
    if (credit < (long long) pow(10, 13) || credit > (long long) pow(10, 16))
    {
        printf("INVALID\n");
        valid = false;
    }


    int buffer = numofdigits(credit);

    long long aux = credit;
    int last_two;
    int last_one;
    //Note: variable names cannot start with numbers!
    int secondlast_one;
    int mul = 1;
    int sum1 = 0;
    int sum2 = 0;

    if(valid){
    for(int i = 0; aux > 0; i++)
    {
        //1st step :
        //Multiply every other digit by 2, starting with the number’s second-to-last digit,
        //and then add those products’ digits together.

        last_two = aux % (long long) pow(10, buffer-2);
        last_one = last_two % 10;
        secondlast_one = last_two / 10;
        mul = secondlast_one*2;

        if((mul / 10) != 0)
        {
            sum1 += (mul / 10) + (mul % 10);
        }
        else
        {
            sum1 += mul;
        }
        //2nd step:
        //Add the previous sum to the sum of the digits that weren’t multiplied by 2.
        sum2 += last_one;

        aux = aux / 100;
    }

    //3rd step:
    //If the total’s last digit is 0
    //(or, put more formally, if the total modulo 10 is congruent to 0),
    //the number is valid!
    if((sum1 + sum2) % 10 ==0)
    {
        valid = true;
    }


    int fst_two = fst_ndigit(credit, buffer, 2);
    int fst_one = fst_ndigit(credit, buffer, 1);

    char str[17];
    // Note :
    // sprintf(str, "%d", (int)credit);
    // this line is incorrect because the prog will spit out a random integer

    sprintf(str, "%lld", credit);
    // lld is format for long long int


    if (digit(str, buffer) && valid)
    {
        if (buffer == 15 && (fst_two == 34 || fst_two == 37))
        {
            printf("AMEX\n");
        }
        else if (buffer == 16 && (fst_two == 51 || fst_two == 52 || fst_two == 53 || fst_two == 54 || fst_two == 55))
        {
            printf("MASTERCARD\n");
        }
        else if ((buffer == 13 || buffer == 16) && fst_one == 4)
        {
            printf("VISA\n");
        }
        else
        {
            printf("INVALID\n");
        }
    }
    else
    {
        printf("INVALID\n");
    }
    }

}


int numofdigits(long long card_num)
{
    int count = 0;
    while (card_num > 0)
    {
        card_num /= 10;
        count++;
    }
    return count;
}

bool digit(string str, int buffer)
{
    bool all_digits = true;
    for (int i = 0; i < buffer; i++)
    {
        if (!isdigit(str[i]))
        {
            all_digits = false;
            break;
        }
    }
    return all_digits;
}

int fst_ndigit(long long credit, int buffer, int numdigit)
{
    int fstndigit = (int) (credit / (long long) pow(10, buffer - numdigit));
    return fstndigit;
}

as you see in the checksum section:

int buffer = numofdigits(credit);

    long long aux = credit;
    int last_two;
    int last_one;
    //Note: variable names cannot start with numbers!
    int secondlast_one;
    int mul = 1;
    int sum1 = 0;
    int sum2 = 0;

    if(valid){
    for(int i = 0; aux > 0; i++)
    {
        //1st step :
        //Multiply every other digit by 2, starting with the number’s second-to-last digit,
        //and then add those products’ digits together.

        last_two = aux % (long long) pow(10, buffer-2);
        last_one = last_two % 10;
        secondlast_one = last_two / 10;
        mul = secondlast_one*2;

        if((mul / 10) != 0)
        {
            sum1 += (mul / 10) + (mul % 10);
        }
        else
        {
            sum1 += mul;
        }
        //2nd step:
        //Add the previous sum to the sum of the digits that weren’t multiplied by 2.
        sum2 += last_one;

        aux = aux / 100;
    }

    //3rd step:
    //If the total’s last digit is 0
    //(or, put more formally, if the total modulo 10 is congruent to 0),
    //the number is valid!
    if((sum1 + sum2) % 10 ==0)
    {
        valid = true;
    }

in a loop that goes on until all digits of the credit card number is checked, I tried to extract the last digit and the second to last then multiplied the latter by 2 and added to sum1 and if it surpasses 9, I add each of its digits indiviually then sum2 is the sum of every other digit beginnig with the last digit

finally out of the loop:

if total = sum1 + sum2 have as last digit 0 then the credit car number is valid other than that invalid

can someone please help me ?

edit:

int buffer = numofdigits(credit);
    char str[17];
    // Note :
    // sprintf(str, "%d", (int)credit);
    // this line is incorrect because the prog will spit out a random integer

    sprintf(str, "%lld", credit);
    // lld is format for long long int

    int sum = 0;
    int len = strlen(str);
    int i;

  // Iterate through the digits from right to left
  for (i = len - 1; i >= 0; i--) {
    int digit = str[i] - '0';

    // Double every second digit
    if ((len - i) % 2 == 0) {
      digit *= 2;

      // Subtract 9 if the doubled digit is greater than 9
      if (digit > 9) {
        digit -= 9;
      }
    }

    sum += digit;
  }
  int checksum = sum % 10;
  valid = (checksum == 0);

still the same problem

2nd edit : this is the code i wrote that passed the cs50 check Thank everyone for your input.

#include <cs50.h>
#include <ctype.h>
#include <math.h>
#include <stdio.h>

bool digit(string str, int buffer);
int numofdigits(long int credit);
int fst_ndigit(long int credit, int buffer, int numdigit);
bool checksum(long int credit);

int main(void)
{
    long int credit = get_long("please enter your credit card number :  ");

    bool valid = true;

    if (credit < (long long) pow(10, 12) || credit > (long long) pow(10, 17))
    {
        printf("INVALID\n");
        valid = false;
    }

    if (valid)
    {
        valid = checksum(credit);
        if (valid)
        {
            int buffer = numofdigits(credit);
            int fst_two = fst_ndigit(credit, buffer, 2);
            int fst_one = fst_ndigit(credit, buffer, 1);

            if (buffer == 15 && (fst_two == 34 || fst_two == 37))
            {
                printf("AMEX\n");
            }
            else if (buffer == 16 && (fst_two == 51 || fst_two == 52 || fst_two == 53 ||
                                      fst_two == 54 || fst_two == 55))
            {
                printf("MASTERCARD\n");
            }
            else if ((buffer == 13 || buffer == 16) && fst_one == 4)
            {
                printf("VISA\n");
            }
            else
            {
                printf("INVALID\n");
            }
        }
        else
        {
            printf("INVALID\n");
        }
    }
}

bool checksum(long int credit)
{
    int digit_count = 0;
    int sum = 0;
    // checksum for the luhn algorithm
    while (credit > 0)
    {
        int digit = credit % 10;
        credit /= 10;
        digit_count++;

        if (digit_count % 2 == 0)
        {
            digit *= 2;
            if (digit > 9)
            {
                digit -= 9;
            }
        }

        sum += digit;
    }
    return (sum % 10) == 0;
}

int numofdigits(long int credit)
{
    int i = 0;
    while (credit > 0)
    {
        credit /= 10;
        i++;
    }
    return i;
}

bool digit(string str, int buffer)
{
    bool all_digits = true;
    for (int i = 0; i < buffer; i++)
    {
        if (!isdigit(str[i]))
        {
            all_digits = false;
            break;
        }
    }
    return all_digits;
}

int fst_ndigit(long int credit, int buffer, int numdigit)
{
    int fstndigit = (int) (credit / (long long) pow(10, buffer - numdigit));
    return fstndigit;
}

Solution

  • At least these issues:

    Wrong way to get least 2 significant digits

    // last_two = aux % (long long) pow(10, buffer-2);
    last_two = aux % 100;
    

    Buffer overflow

    sprintf(str, "%lld", credit); is UB when credit is 10,000,000,000,000,000 or larger. Instead of char str[17];, use char str[100];.

    Instead of optimizing the buffer size and risking buffer overflow. Use a generous buffer size. I suggest 2x or more the anticipated size or write code that will absolutely not overrun.

    OP's code allows credit == 10,000,000,000,000,000 which needs a buffer of 18.

    Avoid floating point for an integer problem

    Rework code to not use pow().