0

Huge thanks to everyone that answered , i have realised that i suck a lot at this, i will take every answer into consideration and hopefully i will manage to compile something that is working

3 Answers3

3

Some remarks:

  1. Allocating 500 MB just in case doesn't seem like a good idea. A better approach would be to allocate a small amount of memory first, if it's not enough then allocate 2 times bigger memory, etc (this would work if you read the number on per-character basis). Important: right after every (re)allocation, you have to check whether your malloc call succeeded (i.e. what it returns is not NULL), otherwise you cannot go any further.
  2. what the first getchar() is for?
  3. instead of using gets(), you could try to read the characters one-by-one, until you encounter something that is not a number, at which point you can assume that the number input has finished (that is the simplest way, obviously one can process user input differently).
  4. adding '\0' for something that was read with gets() is not needed, afaik (for something that would be read character-by-character, that would make sense).
  5. Last but not least, you should also take care of actually freeing the allocated memory (i.e. calling free() after you are done with num). Not doing so results in a memory leak.
  6. (Update) printf("%c",num[0]); will only print the first character of the string num. If you want to print out the whole string, you should call printf("%s",num);
Ashalynd
  • 12,363
  • 2
  • 34
  • 37
  • 2
    _on per-character basis_, better yet, on per-chunk basis. – Shahbaz Mar 11 '14 at 17:52
  • Also, checking for `num == NULL` should be done before using it (`gets`) not after. Furthermore, the addition of `NUL` is meaningless in this case since `strlen` relies on it being there anyway! – Shahbaz Mar 11 '14 at 17:53
  • the first getchar is nothing , i just used it because i used puts(xD) and it gives a \n by default so gets() gives core dump i ll change that . Is this code elligible of reading a big number if i allocate a smaller amount ? thx for the reply – user3407269 Mar 11 '14 at 18:02
  • You can read a string, but the real issue is, if you are going to actually treat it as a number you'll have somehow to convert it into a number, and that's not trivial either. – Ashalynd Mar 11 '14 at 21:07
2

Well, there are quite a few problems with this code, none that necessarily have to do with reading big numbers. But you're still learning, so here we go. In order in which they appear in the code:

  1. (Not really an error, but also not recommended): Casting the result of malloc is unnecessary, as outlined in this answer.
  2. As the other answer states: allocating 500MB is probably way overkill, if you really need this much you can always add more, but you may want to start out with less (5KB, for example).
  3. You should add a new-line at the end of your puts, or the output may end up in places where you don't expect it (i.e. much later).
  4. (This is an error) Don't ever use gets: this page explains why.
  5. You're checking if(num == NULL) after you've already used it (presumably to check if gets failed, but it will return NULL on failure, the num pointer itself won't be changed). You want to move this check up to right after the malloc.
  6. After your NULL-check for num your code happily continues after the if, you'll want to add a return or exit inside the if's body.
  7. There is a syntax error with your very last printf: you forgot the closing ].

When you decide to use fgets to get the user input, you can check if the last character in the string is a new-line. If it isn't then that means it couldn't fit the entire input into the string, so you will need to fgets some more. When the last character is a new-line you might want to remove that (use num[len]='\0'; trick that isn't necessary for gets, but is for fgets).

Instead of increasing the size of your buffer by just 1, you should grow it by a bit more than that: a common used value is to just double the current size. malloc, calloc and realloc are fairly expensive system-calls (performance-wise) and since you don't seem too fussed about memory-usage it can save a lot of time keeping these calls to a minimum.

An example of these recommendations:

size_t bufferSize = 5000, // start with 5K
    inputLength = 0;
char * buffer = malloc(bufferSize);

if(buffer == NULL){
    perror("No memory!");
    exit(-1);
}

while(fgets(buffer, bufferSize, stdin) != NULL){
    inputLength = strlen(buffer);
    if(buffer[inputLength] != '\n'){ // last character was not a new-line
        bufferSize *= 2; // double the buffer in size
        char * tmp = realloc(buffer, bufferSize);

        if(tmp == NULL){
            perror("No memory!");
            free(buffer);
            exit(-1);
        }
        // reallocating didn't fail: continue with grown buffer
        buffer = tmp;
    }else{
        break; // last character was a new-line: were done reading
    }
}

Beware of bugs in the above code; I have only proved it correct, not tried it.

Finally, instead of re-inventing the wheel, you may want to take a look at the GNU Multiple Precision library which is specifically made for handling big numbers. If anything you can use it for inspiration.

Community
  • 1
  • 1
Kninnug
  • 7,992
  • 1
  • 30
  • 42
  • its working perfectly , i cant thank u enough really . i have been learning for 2 weeks now so hopefully thats my excuse for sucking – user3407269 Mar 11 '14 at 18:45
  • @user3407269 No one can become an expert programmer in just 2 weeks, so your programming-sins are quite forgiven. I'm glad my code helped, but I hope you can learn from it (and not just use it as copy-pasta). – Kninnug Mar 11 '14 at 18:51
  • yea , i can understand the code fine and have made my changes it has really helped me to understand more about malloc and stuff thanks again – user3407269 Mar 11 '14 at 18:54
1

This is how you could go about reading some really big numbers in. I have decided on your behalf that a 127 digit number is really big.

#include <stdio.h>
#include <stdlib.h>

#define BUFSIZE 128

int main()
{
    int n, number, len;    

    char *num1 = malloc(BUFSIZE * sizeof (char));
    if(num1==NULL){
        puts("Not enough memory");
        return 1;
    }

    char *num2 = malloc(BUFSIZE * sizeof (char));
    if(num2==NULL){
        puts("Not enough memory");
        return 1;
    }

    puts("Please enter your first number");
    fgets(num1, BUFSIZE, stdin);

    puts("Please enter your second number");
    fgets(num2, BUFSIZE, stdin);

    printf("Your first number is: %s\n", num1);
    printf("Your second number is: %s\n", num2); 

    free(num1);
    free(num2);

    return 0;
}

This should serve as a starting point for you.

Tom Fenech
  • 72,334
  • 12
  • 107
  • 141