My code functions with a printf statement, but not without it

Question

Adding the printf("Hi!\n") statements allows the code to work. It also works if the bound initial bound is improper and the user enters a new one. When I ran some tests calculate divers sometimes returned a character instead of an integer. I'm thinking it has something to do with my memory allocation. I also noticed that ./a.out 6 10 "|" would work but ./a.out 6 25 "|" would not causing an infinite loop when printing the lines of "|".

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

// Structs
typedef struct data_struct {
    int lineNumber;
    int divisorSum;
    char type[10];
}data;

// Prototypes
int calculateDivsors(int integer);

// Functions
int main (int argc, char *argv[]) {

    int lowerBound;
    int upperBound;
    char character;

    // Gets the values from command-line
    sscanf(argv[1], "%d", &lowerBound);
    sscanf(argv[2], "%d", &upperBound);
    sscanf(argv[3], "%c", &character);

    // Check to see if bound is proper
    while (upperBound <= lowerBound || lowerBound < 2) {
        printf("Error, please enter a new range (positive increasing).\n");
        scanf("%d %d", &lowerBound, &upperBound);
    }

    // Structure calls
    data* info = NULL;
    int totalData = upperBound - lowerBound;

    // Allocate the memory
    info = (data*)malloc(totalData * sizeof(data));

    printf("Hi!\n");

    if (info != NULL) {
        // Iterate through all the digits between the two bounds
        for (int i = lowerBound; i <= upperBound; i++) {
            int sum = calculateDivsors(i);


            // Write data to indiviual structures
            info[i].lineNumber = i;
            info[i].divisorSum = sum;

            // Check to see if the sum is greater than, less than, or equal to the original
            if (sum == i) {
                strcpy(info[i].type, "Perfect");
            }

            else if (sum > i) {
                strcpy(info[i].type, "Abundant");
            }

            else if (sum < i) {
                strcpy(info[i].type, "Deficient");
            }

            // Line n# has a column width of 4, string of 10
            printf("%4d is %-10s\t", info[i].lineNumber, info[i].type);

            // Generate Pictogram
            for (int j = 0; j < info[i].divisorSum; j++) {
                printf("%c", character);
            }
            printf("\n");
        }

    }
}

// Adds up the sum of diviors
int calculateDivsors(int integer) {

    int sum = 0; 

    for (int i = 1; i < integer; i++) {
        // Add to sum if perfectly i is a sum of integer
         if (integer % i == 0) {
            sum += i;
        }
    }

 return sum; // Returns the sum of diviors
}

You have at least one out of range causing heap overrun in this code. The index `i` used for `info[i]` indexing of your allocation is not zero based, but the allocation size, `totalData`, *is* (correctly, btw). Therefore, as `i` increases it will breach `totalSize` and in so doing, you're program invokes undefined behavior. Also totalData should be sized as `int totalData = upperBound - lowerBound + 1;` — WhozCraig, Nov 23 '19 at 01:07
Its working for me on my Mac OS terminal. Why would that be? — Codite, Nov 23 '19 at 01:08
Undefined behavior doesn't mean it gets an error. Anything can happen, including the appearance of working. — Barmar, Nov 23 '19 at 01:10
When you have a buffer overrun, sometimes it overwrites something important and causes an error, other times it overwrites something that isn't used so you don't notice the problem. — Barmar, Nov 23 '19 at 01:11
*Its working for me... Why would that be?* See [this answer](https://stackoverflow.com/questions/37087286/c-program-crashes-when-adding-an-extra-int/37087465#37087465) to a similar question. — Steve Summit, Nov 23 '19 at 01:17
You should seriously ask yourself why you're dynamic-allocating *anything* in this program. There is no need. Each iteration is completely independent of the others, and that includes *all* output. Therefore, a single `data` record populated and dumped within each iteration should print what you want, and avoid the mistake you made of mishandling dynamic sizing and indexing in the first place. — WhozCraig, Nov 23 '19 at 01:33
You can never know whether a `sscanf` conversion succeeds or fails without ***checking the return***. — David C. Rankin, Nov 23 '19 at 01:39
BIWOMM, the most famous of all defenses of buggy code: "But it works on MY machine!" — Carey Gregory, Nov 23 '19 at 02:16
@CareyGregory In fairness, though, the exclamation of BIWOMM is not necessarily a defense. It's probably honest confusion: "Okay, so if it's wrong, then why did it work?" (The concept that undefined behavior can do *anything* is *very* hard to get at first.) — Steve Summit, Nov 23 '19 at 03:47
@SteveSummit I completely agree and I admit to having said BIWOMM myself many times. It's a difficult lesson to learn because the reasons for it can be many, and it takes most people years, myself included. — Carey Gregory, Nov 23 '19 at 04:44

score 4 · Answer 1 · edited Nov 23 '19 at 01:54

You are accessing data outside its allocated buffer whenever lowerBound doesn't start with 0.

info[i].lineNumber = i;

Ideally, you should become...

info[i - lowerBound].lineNumber = i;

To ensure that the indexing starts at 0. Further, your window between lowerBound and upperBound is inclusive. That means it includes both ending boundaries. Therefore, totalData is undersized by one element. Even if you fix the indexing problem, your code will still be wrong with this:

int totalData = (upperBound - lowerBound) + 1;

Failing to do both of the above causes your code to invoke undefined behavior (UB), and thus unpredictable results thereafter. It may even appear to work. That, however, is a red herring when your code has UB. Don't confuse defined behavior with observed behavior. You can trust the latter only once you have the former; the two are not synonymous.

My code functions with a printf statement, but not without it

1 Answers1