2

I am developing a c program which connects to a mysql database reads data from a a table of 4 million of data and writes its data to another 100 tables in another database after some calculations. To make it efficient I tried to use 100 threads to write data for 100 tables and 1 thread to read data from the database and write those in to a buffer. So that the 100 threads would read from the buffers.

But the problem is when I'm making the buffers. I used malloc to make the buffers. char *** queue; is declared in the header file so that it is global

int i = 0, j = 0;
queue = (char ***) malloc(100);
int threadRet;

for (i; i < 100; i++) {
    queue[i] = (char **) malloc(2000);
    for (j; j < 2000; j++) {
        queue[i][j] = (char *) malloc(180);
    }
}

and My buffer writing thread as I mentioned before exicutes the function void * thrededChunkPicker(void * parr)

        sprintf(queue[tableNo][marker[tableNo]], "INSERT INTO usage_summary%s(mobile,`0`,ddate) VALUES('%s',%d,'%s') ON DUPLICATE KEY UPDATE `0` = VALUES(`0`), ddate=VALUES(ddate)", row[0] + 7, row[0], sumOfUsage, row[2]);
        (marker[tableNo])++;

This is how I write to the buffer . As I've found out the segmentation fault occurs here.

I needed 100 buffers in each of which contains 2000 string arrays of 180. this code compiled successfully.but when runs it gives a segmentation fault.

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
Laksith
  • 354
  • 3
  • 20
  • any comments on my declarations. I mean my way of mallocing. Is it a good practice – Laksith Jan 14 '16 at 07:39
  • 1
    [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Jan 14 '16 at 07:40
  • Don't strive to be a [three-star programmer](http://c2.com/cgi/wiki?ThreeStarProgrammer). – Some programmer dude Jan 14 '16 at 07:42
  • 1
    Use `for (i = 0; i < 100; i++)` even if you initialize `i` upon definition. As a matter of fact, `for (j; j < 2000; j++)` will do nothing after the first iteration because you do no reset `j` to `0`!. Use `for (j = 0; j < 2000; j++)`. – chqrlie Jan 14 '16 at 07:46

2 Answers2

7

As far as I can see, all your malloc() calls are problematic.

We pass the size in bytes to malloc(). malloc() does not have any idea of the size of the object in which you're going to store the returned pointer, whatsoever. So, it cannot automatically calculate the total size for the memory required.

We need to calculate the total size required and pass that size to malloc().

First to start with, please see this discussion on why not to cast the return value of malloc() and family in C..

That said, assuming queue is defined as char *** queue;, we need to

  • queue = malloc(100 * sizeof*queue );
  • queue[i] = malloc(2000 * sizeof(*queue[i]) );

and so on, if required.

Finally, always check for the success of malloc() through a NULL check on the returned pointer before using the same.

Community
  • 1
  • 1
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • I made the changes but. it did'nt work. The result was same. But thankz anyway I really didn't know that – Laksith Jan 14 '16 at 08:59
  • could you tel me whether my sprintf() 's parameters are correct I mean how the buffer element is written – Laksith Jan 14 '16 at 09:03
  • @Laksith what are the contents of the arguments you've passed? Remember, you need to calculate the size (e.g., 180), with the argument values into the string including the null terminator. Can you check and confirm this? – Sourav Ghosh Jan 14 '16 at 09:17
  • table no is a 0 -99 value. and the string Im passing to the buffer is 150 chars long. `marker[]` is a int array where its elements get incremented from 0- 1999 so I can decrement increment when a value is inserted or deleted – Laksith Jan 14 '16 at 10:54
3

The biggest problem is that malloc allocates a number of bytes, not elements in an array. So you allocate 100 bytes for your three-star variable, which is not enough space for 100 elements. The same with the next call to malloc, it allocates 2000 bytes not 2000 elements.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621