LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (https://www.linuxquestions.org/questions/programming-9/)
-   -   Basic C programming question (https://www.linuxquestions.org/questions/programming-9/basic-c-programming-question-4175719828/)

lxs602 12-15-2022 03:42 PM
Basic C programming question
 
Hi,

I am taking the Harvard CS50 online course. The programme is to check credit card number validity.

Why does the checkfunction() fail with a floating point error, when tested with a valid Mastercard number?

I also tried using gdb, but it gave an error to do with get_long being not declared.

Any other comments on how to improve the code very welcome.

NB. I'm not sure if the maths works yet...

Code:
#include <cs50.h>
#include <stdio.h>

 // x is the input
long x;
 // y is the calculated checksum
int y = 0;
 // i is an integer for calculations
int i = 1;

// Function to loop through the digits of the card number, beginning with the last digit. See HP Luhm algorithm for further details
int checkfunction(void)
{
    printf("made it here also\n");
    //
    for (i = i + 0; i <= x; i *= 1e2)
            {
                //  Divide the number by a multiple of 10 and take the remainder. Multiply the digit by 2 and add to the checksum.
                int z;
                z = x % 10;
                if (z <= 4)
                {
                    y = z * 2 + y;
                }
                // If the product would be >10, add each digit of the product separately to the checksum
                else if (z >= 6)
                {
                    y = ( z * 2 ) - 10 + 1 + y;
                }
                // Add the next digit along (from right to left)
                y = (x % (i * 10)) + y;

            }
    return y;
}

int main(void)
{

    x = get_long("Number: ");

    // Exclude numbers shorter than 13 digits, longer than 16 digits, or that are 14 digits
    if (x < 1e12 || x > 1e16 - 1 || (x >= 1e13 && x <= 1e14))
    {
        printf("INVALID\n");
    }
    else
    {
        // For 15 digit cards
        if (x > 1e14 && x < 1e15 - 1)
        {
        // Check first two digits are 34 or 37, for Amex
        if ((x < 34e13 && x > 1e14 - 1) || (x > 37e13 && x < 38e13 - 1))
        {

            // Loop through the digits of the card number, beginning with the last digit
            checkfunction();

            // If checksum is divisible by 10, then credit card number is valid
            if ((y % 10) == 0)
            {
                printf("AMEX\n");
            }
            else
            {
                printf("INVALID\n");
            }
        }
        }
        // For 13 digit VISA cards, beginning with 4
        else if (x > 4e12 && x < 5e12 - 1)
        {
            // Loop through the digits of the card number, beginning with the last digit
            checkfunction();

            // If checksum is divisible by 10, then credit card number is valid
            if ((y % 10) == 0)
            {
                printf("VISA\n");
            }
            else
            {
                printf("INVALID\n");
            }
        }

        // For 16 digit VISA cards, also beginning with 4
        else if (x > 4e15 && x < 5e15 - 1)
        {
            // For 16 digit cards, set i to 1
            i = 1;
            // Loop through the digits of the card number, beginning with the last digit
            checkfunction();

            // If checksum is divisible by 10, then credit card number is valid
            if ((y % 10) == 0)
            {
                printf("VISA\n");
            }
            else
            {
                printf("INVALID\n");
            }
        }
        // For Mastercards (16 digits), beginning with 51, 52, 53, 54 or 55
        else if (x > 51e14  && x < 56e14 - 1)
        {
            printf("made it to this point\n");

            // Loop through the digits of the card number, beginning with the last digit
            checkfunction();
            printf("%i", y);

            // If checksum is divisible by 10, then credit card number is valid
            if ((y % 10) == 0)
            {
                printf("MASTERCARD\n");
            }
            else
            {
                printf("INVALID\n");
            }
        }
        else
        {
            printf("INVALID\n");
        }
    }
}

teckk 12-15-2022 03:53 PM
Line 40
Code:
x = get_long("Number: ");

lxs602 12-15-2022 04:04 PM
In the wrong place / incorrectly assigned?

teckk 12-15-2022 04:07 PM
I downloaded that header file from here
https://raw.githubusercontent.com/cs...ain/src/cs50.h

I included it with
Code:
gcc test.c -o test -I/home/me/
And sure enough
Code:
gcc test.c -o test -I/home/me/
/usr/bin/ld: /tmp/ccJBimDQ.o: in function `main':
aaa.c:(.text+0x12b): undefined reference to `get_long'
collect2: error: ld returned 1 exit status
You don't have get_long defined anywhere. Without more info, that is all that one can tell you.

teckk 12-15-2022 04:12 PM
I replaced:
Code:
//x = get_long("Number: ");
printf("Number: \n");
scanf("%d", &x);
And it compiled and runs.
Code:
./test
Number:
12345678
INVALID
I don't know what get_long is suppose to do.

rnturn 12-15-2022 04:57 PM
Quote:
Originally Posted by lxs602 (Post 6398140)
In the wrong place / incorrectly assigned?
What happens if you move the declaration of "x" down into the top of main() rather than declaring it at the top of the entire file? (Caveat: my C is rusty but I've never been keen on using variables that look to be global. My feelings won't be hurt if this doesn't help.)

GazL 12-15-2022 05:48 PM
Use of those floating point literals makes me nervous. Not all numbers can be represented as a floating point value and using them forces the implicit conversion of 'x' into a double for any expressions using one of them as an operand. Much safer to stick with longs.

The first positive integer number that is unrepresentable as a 64bit double is over 9e+15, so most card numbers are likely safe, but "1e16 - 1" might not be!
Lets see...
Code:
$ cat float.c
#include <stdio.h>

int main()
{
        printf("%f\n", 1e16 - 1 );
        printf("%f\n", 56e14 - 1 );

        return 0;
}
$ cc -Wall float.c && ./a.out
10000000000000000.000000
5599999999999999.000000
$
As you can see, the 1e16 - 1 gets rounded back up to 1e16.


So, if your only goal for using the exponent form is to make the code more readable by avoiding those long numbers cluttering up your expressions then you'd be much better off using a macro instead, something like:
#define L_1e14 100000000000000L

This will keep the code readable while avoiding all the nastiness that using floating point operations introduces.

GazL 12-15-2022 05:59 PM
Quote:
Originally Posted by teckk (Post 6398141)
You don't have get_long defined anywhere. Without more info, that is all that one can tell you.
As it happens, I can remember watching part of the CS50 video lectures a few weeks back. I think the get_long() is just a wrapper around a simple scanf() with a little error catching thrown in to hide some of the ugliness from the beginner students.

For the purpose of testing, a scanf("%ld", &x); will likely suffice.

ntubski 12-15-2022 06:28 PM
Quote:
Originally Posted by teckk (Post 6398141)
I downloaded that header file from here
https://raw.githubusercontent.com/cs...ain/src/cs50.h
I downloaded https://github.com/cs50/libcs50/blob/v11.0.1/src/cs50.c as well, I'm able to reproduce the crash with a large enough number:

Code:
$ ./check-credit
Number: 4242424242424242
made it here also
Floating point exception
Quote:
Originally Posted by lxs602 (Post 6398133)
I also tried using gdb, but it gave an error to do with get_long being not declared.
Do you not have the headers in the right place? It's really important to get a debugger working for problems like this:

Code:
Program received signal SIGFPE, Arithmetic exception.
0x00005555555552fe in checkfunction () at check-credit.c:31
31                      y = (x % (i * 10)) + y;
(gdb) print i
$4 = -2147483648
(gdb) print/x i
$5 = 0x80000000

sundialsvcs 12-16-2022 11:45 AM
I would design this function to use a string input which it parses character by character, converting each to a digit with x = ord(ch) - ord('0'); after testing that the character is, in fact, a digit. This would allow the input of spaces which the loop could simply ignore. You can count the digits to ensure that the length is correct.

Your code as-written assumes that a credit card number can be represented as long but this is not a valid assumption.

teckk 12-16-2022 01:59 PM
Quote:
I think the get_long() is just a wrapper around a simple scanf() with a little error catching...
Oh ok, get_long is here, finally had a chance to look at it.

https://github.com/cs50/libcs50/blob/v11.0.1/src/cs50.c
Line 369

teckk 12-16-2022 02:27 PM
Ok, to put all of that into one c file, so that you can look at it, and study it, without the need to include header files from a different location, and to see what it is doing.

aaa.c
Code:
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <ctype.h>
#include <float.h>
#include <limits.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <limits.h>
#include <math.h>
#include <stdint.h>
#include <stdbool.h>
#include <stdlib.h>

//#define _GNU_SOURCE

long x;
int y = 0;
int i = 1;

typedef char *string;
static size_t allocations = 0;
static string *strings = NULL;
char get_char(const char *format, ...) __attribute__((format(printf, 1, 2)));
double get_double(const char *format, ...) __attribute__((format(printf, 1, 2)));
float get_float(const char *format, ...) __attribute__((format(printf, 1, 2)));
int get_int(const char *format, ...) __attribute__((format(printf, 1, 2)));
long get_long(const char *format, ...) __attribute__((format(printf, 1, 2)));
long long get_long_long(const char *format, ...) __attribute__((format(printf, 1, 2)));
string get_string(va_list *args, const char *format, ...) __attribute__((format(printf, 2, 3)));

string get_string(va_list *args, const char *format, ...) {
    if (allocations == SIZE_MAX / sizeof (string)) {
        return NULL;
    }
    string buffer = NULL;
    size_t capacity = 0;
    size_t size = 0;
    int c;
   
    if (format != NULL) {
        va_list ap;
        if (args == NULL) {
            va_start(ap, format);
        } else {
            va_copy(ap, *args);
        }
        vprintf(format, ap);
        va_end(ap);
    }

    while ((c = fgetc(stdin)) != '\r' && c != '\n' && c != EOF) {
        if (size + 1 > capacity) {
            if (capacity < SIZE_MAX) {
                capacity++;
            } else {
                free(buffer);
                return NULL;
            }
            string temp = realloc(buffer, capacity);
            if (temp == NULL){
                free(buffer);
                return NULL;
            }
            buffer = temp;
        }
        buffer[size++] = c;
    }

    if (size == 0 && c == EOF) {
        return NULL;
    }

    if (size == SIZE_MAX) {
        free(buffer);
        return NULL;
    }

    if (c == '\r' && (c = fgetc(stdin)) != '\n') {
        if (c != EOF && ungetc(c, stdin) == EOF) {
            free(buffer);
            return NULL;
        }
    }

    string s = realloc(buffer, size + 1);
    if (s == NULL) {
        free(buffer);
        return NULL;
    }
    s[size] = '\0';
    string *tmp = realloc(strings, sizeof (string) * (allocations + 1));
    if (tmp == NULL) {
        free(s);
        return NULL;
    }
    strings = tmp;
    strings[allocations] = s;
    allocations++;
    return s;
}

long get_long(const char *format, ...) {
    va_list ap;
    va_start(ap, format);
   
    while (true) {
        string line = get_string(&ap, format);
       
        if (line == NULL) {
            va_end(ap);
            return LONG_MAX;
        }

        if (strlen(line) > 0 && !isspace((unsigned char) line[0])) {
            char *tail;
            errno = 0;
            long n = strtol(line, &tail, 10);
           
            if (errno == 0 && *tail == '\0' && n < LONG_MAX) {
                va_end(ap);
                return n;
            }
        }
    }
}

int checkfunction(void) {
    printf("Made it to point A\n");  //Point A

    for (i = i + 0; i <= x; i *= 1e2) {
        int z;
        z = x % 10;
       
        if (z <= 4) {
            y = z * 2 + y;
        } else if (z >= 6) {
            y = ( z * 2 ) - 10 + 1 + y;
        }
        y = (x % (i * 10)) + y;

    }
    return y;
}

int main(void)
{
    x = get_long("Number: ");
   
    if (x < 1e12 || x > 1e16 - 1 || (x >= 1e13 && x <= 1e14)) {
        printf("INVALID A\n"); //Inv A
    } else {
        if (x > 1e14 && x < 1e15 - 1) {
            if ((x < 34e13 && x > 1e14 - 1) || (x > 37e13 && x < 38e13 - 1)) {
                checkfunction();
               
                if ((y % 10) == 0) {
                    printf("AMEX\n");
                } else {
                    printf("INVALID B\n"); //Inv B
                }
            }
        } else if (x > 4e12 && x < 5e12 - 1) {
            checkfunction();
           
            if ((y % 10) == 0) {
                printf("VISA\n");
            } else {
                printf("INVALID C\n"); //Inv C
            }
        } else if (x > 4e15 && x < 5e15 - 1) {
            i = 1;
            checkfunction();

            if ((y % 10) == 0) {
                printf("VISA\n");
            } else {
                printf("INVALID D\n"); //Inv D
            }
        } else if (x > 51e14  && x < 56e14 - 1) {
            printf("Made it to this point B\n"); //Point B
            checkfunction();
            printf("%i", y);

            if ((y % 10) == 0) {
                printf("MASTERCARD\n");
            } else {
                printf("INVALID E\n"); //Inv E
            }
        } else {
            printf("INVALID F\n"); //Inv F
        }
    }
}
Code:
gcc aaa.c -o aaa
Code:
./aaa
Number: 111111111111
INVALID A

./aaa
Number: 1111111111111
INVALID F

./aaa
Number: 111111111111111
Made it to point A
Floating point exception (core dumped)

sundialsvcs 12-16-2022 07:26 PM
Let me kindly repeat: your true input is a string, which must consist of digit-characters and spaces, and which must contain a specified number of digit-characters. Do not try to use a library call to convert this input into "an integer" of whatever length, because in the general case it will not work. You need to step back and reconsider your entire approach.

teckk 12-17-2022 05:24 AM
https://codereview.stackexchange.com...alidation-in-c
https://stackoverflow.com/questions/...ard-validation
https://www.geeksforgeeks.org/progra...er-validation/
https://github.com/PouletEnSlip/CreditCardVerification
https://github.com/mendelgordon/credit-card-checker
https://github.com/acmpesuecc/CreditCardVerifier

NevemTeve 12-17-2022 10:43 AM
Code:
$ diff -u cs50.bak cs50.c
--- cs50.bak    2022-12-17 17:26:48.111099200 +0100
+++ cs50.c      2022-12-17 17:41:56.737257200 +0100
@@ -6,7 +6,7 @@
  // y is the calculated checksum
 int y = 0;
  // i is an integer for calculations
-int i = 1;
+long int i = 1; /* move this definition into 'checkfunction' */

 // Function to loop through the digits of the card number, beginning with the last digit. See HP Luhn algorithm for further details
 int checkfunction(void)

GazL 12-19-2022 11:05 AM
Seems most of them make the same mistake that Sundial was warning about: a long is not guaranteed to be large enough on all platforms. unsigned long long should be good for 19 digits however, but as Sundial says, why limit yourself: process the number as a string.

lxs602 02-06-2024 02:30 PM
Long overdue, but thanks.

sundialsvcs 02-06-2024 10:34 PM
Also: when this is not “a learning exercise,” remember that there are many contributed libraries of software … for every programming language that exists … where other(!) people have confronted these “problems that we all have,” and solved them very well. Including all(!) variations of “credit-card number validation.”

It’s very important to realize, when you are first “learning to program in [X],” that today you are not “alone.” Extremely the opposite. Therefore, when you confront any problem, your first assumption should be that someone else has already done it. (And that you can freely “sneak a peek” at their source code. In due time, you might make your own contribution …)

lxs602 04-23-2024 03:23 PM
Thanks. I was trying not to, but probably better than becoming frustrated.

murugesandins 04-23-2024 08:55 PM
Quote:
Originally Posted by lxs602 (Post 6398133)
I also tried using gdb, but it gave an error to do with get_long being not declared.
I faced the same issue after copy and compiling your code.c at localhost using gcc.exe (CYGWIN_NT)
Initially I was not having libcs50 library
Downloaded https://github.com/cs50/libcs50/arch...gs/v11.0.2.zip
Code:
$ unzip.exe libcs50-11.0.2.zip >/dev/null 2>&1
$ cd libcs50-11.0.2
$ make
make: Nothing to be done for 'all'.
$ /usr/bin/cp -i "Makefile" "Makefile.Original"
$# I modified Makefile related to CYGWIN
$ diff Makefile Makefile.Original
17c17
< OS := $(shell /usr/bin/uname | /usr/bin/sed "s/-[0-9]*\.[0-9]-[0-9]*//;")
---
> OS := $(shell uname)
19,24d18
< # CYGWIN_NT
< ifeq ($(OS),CYGWIN_NT)
<      LIB_BASE := $(BASENAME).so
<      LIB_MAJOR := $(BASENAME).so.$(MAJOR_VERSION)
<      LIB_VERSION := $(BASENAME).so.$(VERSION)
<      LINKER_FLAGS := -Wl,-soname,$(LIB_MAJOR)
26c20
< else ifeq ($(OS),Linux)
---
> ifeq ($(OS),Linux)
$ make
#...
install -m 644 src/cs50.h build/include
mv libcs50.so.11.0.2 libcs50.so libcs50.a build/lib
$ /usr/bin/find ./ -type f -name cs50.h
$ mv ./build/include/cs50.h /usr/include/
$ find ./ -type f -name "*.a" -exec grep -l get_long {} \;
./build/lib/libcs50.a
$ find ./ -type f -name "*.so*" -exec grep -l get_long {} \;
./build/lib/libcs50.so.11.0.2
$ mv ./build/lib/libcs50.so.11.0.2 ./build/lib/libcs50.a /usr/lib
$ /usr/bin/gcc.exe -I. -g -Wall 4175719828.c -o ./a.out -lcs50
Inside checkfunction
if( 0 == x || 0 > i )
{
break;
}
to prevent core dump issue
I have taken base version from https://github.com/jhustles/c_verify...editCardType.c
Code:
#include <cs50.h>
#include <math.h>
#include <stdio.h>
int main(void)
{
        // Always initialize the variable
        long DebitCreditCardNum = 0;
        int sumOfProductDigits = 0;
        int sumOfNonProductDigits = 0; // sum of digits that are not products
        int checkSum = 0;
        long NumModulusTen = 0;
        long SecondModulusTen = 0;
        // Prompt user for credit card input.
        do
        {
                DebitCreditCardNum = get_long( "Number: " );
                // printf( "%li\n", DebitCreditCardNum);
        }
        while( 0 > DebitCreditCardNum );
        sumOfProductDigits = 0;
        sumOfNonProductDigits = 0;
        long ccNumbers1 = DebitCreditCardNum; // Used for checkSum
        long ccNumbers2 = DebitCreditCardNum; // Used for card validation
        while( 0 < ccNumbers1 )
        {
                NumModulusTen = ccNumbers1 % 10; // out: 4
                sumOfNonProductDigits += NumModulusTen; // add 4
                // printf( "Adding %li\n to sumOfNonProductDigits\n", NumModulusTen);
                ccNumbers1 = ccNumbers1 - NumModulusTen; // minus the last digit
                ccNumbers1 = ccNumbers1 / 10; // reduce by one last digit by dividing by 10
                // printf( "Updated ccNumbers1 after is: %li\n", ccNumbers1);
                // printf( "###########################\n" );
                // Second Number to iterate thru
                SecondModulusTen = ccNumbers1 % 10; // out: 1
                // printf( "Here's SecondModulusTen: %li\n \n", SecondModulusTen);
                // printf( "Sum Of Product Digits: %i\n \n", sumOfProductDigits);
                // printf( "Sum Of Non-Product Digits: %i\n \n", sumOfNonProductDigits);
                ccNumbers1 = ccNumbers1 - SecondModulusTen;
                ccNumbers1 /= 10;
                if( 9 < SecondModulusTen * 2 ) // if there's two digits you'll have to add each digit
                {
                        // Product result is double digits. Repeat the same thing to split numbers
                        // and reduce the ccNumbers1
                        SecondModulusTen = SecondModulusTen * 2;
                        int numSlice = SecondModulusTen % 10; // split by getting the last digit
                        sumOfProductDigits += numSlice;
                        sumOfProductDigits += ((SecondModulusTen - numSlice) / 10); // add to products sum
                }
                else
                {
                        // multiply the digit by 2 and add to sumOfProductDigits
                        SecondModulusTen = SecondModulusTen * 2;
                        sumOfProductDigits += SecondModulusTen;
                }
        }
        checkSum = sumOfProductDigits + sumOfNonProductDigits;
        // printf( "CHECK SUM: %i\n", checkSum);
        // Algorithm to check if card is valid:
        if( 0 == checkSum % 10 )
        {
                bool amex = false;
                bool mastercard = false;
                bool visa = false;
                amex =
                        (ccNumbers2 >= 340000000000000 && ccNumbers2 < 350000000000000) ||
                        (ccNumbers2 >= 370000000000000 && ccNumbers2 < 380000000000000);
                mastercard =
                        (ccNumbers2 >= 5100000000000000 && ccNumbers2 < 5600000000000000);
                visa =
                        (ccNumbers2 >= 4000000000000 && ccNumbers2 < 5000000000000) ||
                        (ccNumbers2 >= 4000000000000000 && ccNumbers2 < 5000000000000000);
                if(amex)
                {
                        printf( "AMEX\n" );
                }
                else if(mastercard)
                {
                        printf( "MASTERCARD\n" );
                }
                else if(visa)
                {
                        printf( "VISA\n" );
                }
                else
                {
                        printf( "%lu INVALID_NUMBER\n", DebitCreditCardNum );
                }
        }
        else
        {
                printf( "%lu INVALID NUMBER\n", DebitCreditCardNum );
        }
        return 0;
}
/*
01. Always initialize variables
02. Alwayse beter to write return/return0 based on the functions.
03. Replace:
        while (DebitCreditCardNum < 0);
        ...
        if
        ...
        else if
        ...
        for
With:
        while( 0 > DebitCreditCardNum );
        ...
        if
        ...
        else if
        ...
        for
Reason: new programmers may assign using:
        while( DebitCreditCardNum = 0 );
I have taken following ATM card numbers from internet images:
$ ./a.out
Number: 5333619503715702
MASTERCARD
$ a.out
Number: 5555555555554444
MASTERCARD
$ a.out
Number: 5454545454545454
MASTERCARD
$ a.out
Number: 4444333322221111
VISA
*/
$ mv verifyCreditCardType.c ValidateCardType.c
$ /usr/bin/gcc.exe -g -Wall ValidateCardType.c -o ./a.out -lcs50


All times are GMT -5. The time now is 09:41 AM.