1

I'm a beginner in C and after long time and effort I finished my 1500 rows code in C. It was running perfectly and producing results as expected, but when I tried to add more input than the tests I faced a breakdown.

More specifically, I managed to find the source of the problem in the struct array.

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

#define ROWS 5000
#define MAXLOGGERS 26

struct data
    {
        int day;
        int month;
        int year;
        int hour;
        int minute;
        int second;
        float value;
        float temp;
        float hum;
    };  

int main()
{
    printf("Enter the number of loggers that you want to import: ");
    int n = 10;

    while (n > MAXLOGGERS)
    {
        printf("You can input data from maximum %d loggers: ", MAXLOGGERS);
        scanf("%d", &n);
    }

    struct data mydata[n+1][ROWS];

    printf("\n\nSuccess!!\n");
}

Now when I use n = 10 or less, the program finishes as expected. When I change n to 11 or more it crashes.

I'm suspecting something is wrong with the declaration of the struct array, but I can really not figure it out.

Any help would be more than welcome! Thanx (:

  • 1
    `after long time and effort I finished my 1500 rows code in C.`...well, think again of your learning approach. – Sourav Ghosh Apr 12 '17 at 08:56
  • 1
    `while (n > MAXLOGGERS)` is false for anything `n < 26`, so how do you enter the input exactly? – Sourav Ghosh Apr 12 '17 at 08:57
  • _Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a [mcve]._ – Sourav Ghosh Apr 12 '17 at 08:59
  • Check your max heap size value – user3811082 Apr 12 '17 at 09:00
  • Did you saw that you allocate `5000 * (n + 1 ) * ( sizeof(int) * 6 + 3 * sizeof(float))` around : 5000 * 12 * ( 2 * 6 + 3 * 4 ) = **1 440 000 bytes**. [Estimated size of int and float, which may change for each system]. Check the max size you can allocate ;) – kaldoran Apr 12 '17 at 09:01
  • @Sourav Ghoshwhile: (n > MAXLOGGERS) {Prompts you to use n < 26} I guess the desired behavior is not to crash? :D –  Apr 12 '17 at 09:05
  • @user3811082: How do I do that? –  Apr 12 '17 at 09:10
  • @kaldoran: Yes I know, I have in total 15 loggers (want to make the code run for up to 26) and each one has usually about 4100 rows (I used 5000 just to be sure), but is there a restriction in the maximum size to allocate? And can I change that? –  Apr 12 '17 at 09:10
  • @Segmentationfault Large allocations are typically better on the heap, not on the stack. Operating systems might impose limits on stack size, but more rarely on heap usage. Use `malloc()`. – unwind Apr 12 '17 at 09:11
  • look here http://stackoverflow.com/questions/14697170/program-heap-size – user3811082 Apr 12 '17 at 09:12
  • As @unwind point out, there is a limit on the stack, see http://lists.gnu.org/archive/html/bug-coreutils/2009-10/msg00262.html for example. Btw, if you use 4100 rows, you still allocated 5000 ^^ – kaldoran Apr 12 '17 at 09:15

1 Answers1

0
struct data mydata[n+1][ROWS];

You are allocating a very large (two-dimensional) array on the stack, which is consumed up entirely with n >= 11 (on my system, too, leading to a stack overflow which shows up as a segmentation fault).

For such large data, you should rather allocate memory on the heap. This worked fine (on my machine at least):

//struct data mydata[n+1][ROWS];
struct data* mydata = (struct data*)malloc(sizeof(struct data) * (n+1) * ROWS);
printf("\n\nSuccess!!\n");
free(mydata);

Only, access to the array is not that nice any more, as you cannot use the double index operator. You'd have to calculate your offsets yourself:

// mydata[x][y]; gets: 
mydata[x*ROWS + y];

Do not oversee the call to free - every data malloced should be freed again. Getting used to this principle right from the start (thinking of free right the same time when typing malloc) will help you in preventing memory leaks.

As hinted in the comments (and answers referenced from), stack size is normally limited, whereas the only limits imposed for the heap is the size of the (yet unused) physical memory of your machine, which (typically) is far larger. If you happen to run out of system memory, malloc won't crash but return a null pointer. So you actually should check for:

struct data* mydata = (struct data*)malloc(sizeof(struct data) * (n+1) * ROWS);
if(mydata)
{
    printf("\n\nSuccess!!\n");
    free(mydata);
}
else
{
    printf("\n\nOut of memory!!\n");
}
Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • You should avoid casting the return of the malloc http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc. And as you do, always free the memory that have been allocated [We are not doing Java here] – kaldoran Apr 12 '17 at 09:25
  • @kaldoran Well, there are different oppinions about doing the cast or not (showing 216 upvotes for the answer 'why you should do it'...). I personally (admitted, coming from C++) adhere the opinion of the 'do cast' fraction (admitted again: a minority...). – Aconcagua Apr 12 '17 at 09:31
  • Well indeed, then the user will choose to do so or not ^^ – kaldoran Apr 12 '17 at 09:33
  • Thanx, this is working very well! Unfortunately, even though I changed all my mydata[x][y] to mydata[x*(n+1) + y]; the results I get are not as they were before, data from one x are getting overwritten on the other. This makes sense because before I used the mydata[x][y].hour/minute/second to calibrate loggers so they all start from the same time, so I need to change a big part of the code to compete for the lack of a 2x matrix. Any idea if there is a way just to increase the stack capacity, so I do not need to run all over it again? –  Apr 12 '17 at 10:00
  • `x*(n+1)` was just to illustrate - instead, I would increase n first and use the increased value further on: `++n; malloc([...] * n * [...]); array[x*n+y];` - if you need both (n, n+1), you could use a second variable. You should not get a difference compared to `[x][y]`, actually, the latter is under the hoods transformed to the former by the compiler anyway... – Aconcagua Apr 12 '17 at 11:56
  • @Segmentationfault Unfortunately, I there was an error in my answer: it must, of course, be `array[x * ROWS + y]` for the index access. This is the reason for "one x overwriting another one"! Sorry for that... – Aconcagua Apr 12 '17 at 11:58
  • Explanation: You "wrap" n+1 arrays, each of size ROWS, one after another into a large array. Distance from one wrapped array to the next one is, of course, the size of such a wrapped array, which is ROWS, thus `x*ROWS + y`. – Aconcagua Apr 12 '17 at 12:02
  • @Aconcagua Not a problem! It's a mathematical issue after all how will I manipulate my 1x matrix in place of the 2x. Probably thinking something like assigning the first 5000 values to each n, exactly as you said.I'm just a bit too tired to do the changes right now. Thank you very much for your help! –  Apr 12 '17 at 12:07
  • Works like a charm! –  Apr 12 '17 at 16:40