Velocity Reviews > Memory Corruption Problem

# Memory Corruption Problem

August Karlstrom
Guest
Posts: n/a

 03-01-2006
Hi,

Inspired by the recent post by "questions?" I started to write a simple
matrix library. My problem is that when I run the program below I get
unexpected results.

\$ cat test.c
#include <stdio.h>
#include <stdlib.h>

#define SIZE 2

struct matrix
{
int rows, cols;
double **at;
};
typedef struct matrix Matrix;

Matrix *new_matrix(int rows, int cols, const double *elements)
{
Matrix *res;
int j, k;

res = malloc(sizeof (Matrix));
res->rows = rows;
res->cols = cols;
res->at = malloc(rows * sizeof (double *));
for (j = 0; j < rows; j++) {
res->at[j] = malloc(cols * sizeof (double *));
}
if (elements != NULL) {
for (j = 0; j < rows; j++) {
for (k = 0; k < cols; k++) {
res->at[j][k] = elements[j * cols + k];
}
}
}
return res;
}

int main(void)
{
Matrix *A, *B;
double a[SIZE * SIZE] = {1.0, 2.0, 3.0, 4.0};

A = new_matrix(SIZE, SIZE, a);
printf("%f\n", A->at[1][1]);
B = new_matrix(SIZE, SIZE, a);
printf("%f\n", A->at[1][1]);
return 0;
}
\$ gcc -ansi -pedantic -std=c89 -Wall -g test.c module.c -o test
\$ ./test
4.000000
0.000000

I expect the output to be

4.000000
4.000000

Any clues?

Regards,

August

--
I am the "ILOVEGNU" signature virus. Just copy me to your
signature. This email was infected under the terms of the GNU

Xavier
Guest
Posts: n/a

 03-01-2006
August Karlstrom wrote:
> Hi,
>
> Inspired by the recent post by "questions?" I started to write a simple
> matrix library. My problem is that when I run the program below I get
> unexpected results.
>
> \$ cat test.c
> #include <stdio.h>
> #include <stdlib.h>
>
> #define SIZE 2
>
> struct matrix
> {
> int rows, cols;
> double **at;
> };
> typedef struct matrix Matrix;

or all in one typedef struct matrix {...} Matrix;
>
> Matrix *new_matrix(int rows, int cols, const double *elements)
> {
> Matrix *res;
> int j, k;
>
> res = malloc(sizeof (Matrix));
> res->rows = rows;
> res->cols = cols;
> res->at = malloc(rows * sizeof (double *));
> for (j = 0; j < rows; j++) {
> res->at[j] = malloc(cols * sizeof (double *));

res->at[j] = malloc(cols * sizeof (double));
> }
> if (elements != NULL) {
> for (j = 0; j < rows; j++) {
> for (k = 0; k < cols; k++) {
> res->at[j][k] = elements[j * cols + k];

sure ? it seems to be incoherent...
res->at[j][k] = elements[j * rows + k];
> }
> }
> }
> return res;
> }
>
>
> int main(void)
> {
> Matrix *A, *B;
> double a[SIZE * SIZE] = {1.0, 2.0, 3.0, 4.0};
>
> A = new_matrix(SIZE, SIZE, a);
> printf("%f\n", A->at[1][1]);
> B = new_matrix(SIZE, SIZE, a);
> printf("%f\n", A->at[1][1]);
> return 0;
> }
> \$ gcc -ansi -pedantic -std=c89 -Wall -g test.c module.c -o test
> \$ ./test
> 4.000000
> 0.000000
>
> I expect the output to be
>
> 4.000000
> 4.000000
>
> Any clues?
>
>
> Regards,
>
> August
>

Xavier

Xavier
Guest
Posts: n/a

 03-01-2006
Xavier wrote:
> August Karlstrom wrote:
>
>> Hi,
>>
>> Inspired by the recent post by "questions?" I started to write a
>> simple matrix library. My problem is that when I run the program below
>> I get unexpected results.
>>
>> \$ cat test.c
>> #include <stdio.h>
>> #include <stdlib.h>
>>
>> #define SIZE 2
>>
>> struct matrix
>> {
>> int rows, cols;
>> double **at;
>> };
>> typedef struct matrix Matrix;

>
> or all in one typedef struct matrix {...} Matrix;
>
>>
>> Matrix *new_matrix(int rows, int cols, const double *elements)
>> {
>> Matrix *res;
>> int j, k;
>>
>> res = malloc(sizeof (Matrix));
>> res->rows = rows;
>> res->cols = cols;
>> res->at = malloc(rows * sizeof (double *));
>> for (j = 0; j < rows; j++) {
>> res->at[j] = malloc(cols * sizeof (double *));

>
> res->at[j] = malloc(cols * sizeof (double));
>
>> }
>> if (elements != NULL) {
>> for (j = 0; j < rows; j++) {
>> for (k = 0; k < cols; k++) {
>> res->at[j][k] = elements[j * cols + k];

>
> sure ? it seems to be incoherent...
> res->at[j][k] = elements[j * rows + k];

you should put *values* in your array, isn't it?
*(res->at[j]+k) = *(elements+j * rows + k);
>
>> }
>> }
>> }
>> return res;
>> }
>>
>>
>> int main(void)
>> {
>> Matrix *A, *B;
>> double a[SIZE * SIZE] = {1.0, 2.0, 3.0, 4.0};
>>
>> A = new_matrix(SIZE, SIZE, a);
>> printf("%f\n", A->at[1][1]);
>> B = new_matrix(SIZE, SIZE, a);
>> printf("%f\n", A->at[1][1]);
>> return 0;
>> }
>> \$ gcc -ansi -pedantic -std=c89 -Wall -g test.c module.c -o test
>> \$ ./test
>> 4.000000
>> 0.000000
>>
>> I expect the output to be
>>
>> 4.000000
>> 4.000000
>>
>> Any clues?
>>
>>
>> Regards,
>>
>> August
>>

>
> Xavier

Pedro Graca
Guest
Posts: n/a

 03-01-2006
August Karlstrom wrote:
> \$ cat test.c

[snip]
> res->at = malloc(rows * sizeof (double *));

res->at = malloc(rows * sizeof *(res-at));

> for (j = 0; j < rows; j++) {
> res->at[j] = malloc(cols * sizeof (double *));

res->at[j] = malloc(cols * sizeof *(res->at[j]));

> }

[snip]

> Any clues?

After a couple errors like that I always do

POINTER = malloc(ELEMS * sizeof *POINTER);

I tried your code as you posted it and got the
4.000000
0.000000
output. With the corrections noted above the output was what you
expected.

You should also free the allocated memory before exiting.

<OT>
If not free'd by the program will the OS free it?
</OT>

--

Jack Klein
Guest
Posts: n/a

 03-01-2006
On 1 Mar 2006 02:17:02 GMT, Pedro Graca <(E-Mail Removed)> wrote in
comp.lang.c:

[snip]

> You should also free the allocated memory before exiting.
>
> <OT>
> If not free'd by the program will the OS free it?
> </OT>

Some operating systems do, some do not. Most modern ones do, there
were certainly some older ones that did not.

The C standard has no control over operating systems, even operating
systems on which one might wish to execute C programs. So it cannot
impose requirements on what an operating system does before or after a
C program executes.

An early language decision, eventually codified in the standard, was
that normal termination of a program (return from main() or a call to
exit()) will close all open files, which includes flushing any
buffered data to the OS.

Another very specific decision was that normal program termination
would not be required to free allocated memory, and so most
implementations do not.

There is nothing preventing an implementation from walking its memory
allocation tables and freeing any still allocated memory on normal
termination. This is assuming that the implementation has retained
sufficient bookkeeping information to do so.

But once an implementation yields control back to the platforms by
returning a termination status, neither the executable nor the C
standard controls what the system does.

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://c-faq.com/
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

August Karlstrom
Guest
Posts: n/a

 03-01-2006
Xavier wrote:
> August Karlstrom wrote:

[snip]
>> struct matrix
>> {
>> int rows, cols;
>> double **at;
>> };
>> typedef struct matrix Matrix;

>
> or all in one typedef struct matrix {...} Matrix;

or just `typedef struct {} Matrix;' A style issue.

>> Matrix *new_matrix(int rows, int cols, const double *elements)
>> {
>> Matrix *res;
>> int j, k;
>>
>> res = malloc(sizeof (Matrix));
>> res->rows = rows;
>> res->cols = cols;
>> res->at = malloc(rows * sizeof (double *));
>> for (j = 0; j < rows; j++) {
>> res->at[j] = malloc(cols * sizeof (double *));

>
> res->at[j] = malloc(cols * sizeof (double));

Yes, there was the bug. Thanks.

>> }
>> if (elements != NULL) {
>> for (j = 0; j < rows; j++) {
>> for (k = 0; k < cols; k++) {
>> res->at[j][k] = elements[j * cols + k];

>
> sure ? it seems to be incoherent...
> res->at[j][k] = elements[j * rows + k];

No, `elements' contain the matrix rows (this should of course be
documented). There are j * cols elements above the row with index j.
Then we add k to get to the column.

August

--
I am the "ILOVEGNU" signature virus. Just copy me to your
signature. This email was infected under the terms of the GNU

Ian Collins
Guest
Posts: n/a

 03-01-2006
Pedro Graca wrote:
> You should also free the allocated memory before exiting.
>
> <OT>
> If not free'd by the program will the OS free it?

Well that depends on the OS! But yes, a typical OS will, otherwise
you'd have a big problem with memory leeks when programs exit abnormally.
> </OT>
>

--
Ian Collins.

August Karlstrom
Guest
Posts: n/a

 03-01-2006
Xavier wrote:
> Xavier wrote:

[snip]
>> res->at[j][k] = elements[j * rows + k];

>
> you should put *values* in your array, isn't it?
> *(res->at[j]+k) = *(elements+j * rows + k);

Different syntax, same semantics. I find subscripts less obscure.

August

--
I am the "ILOVEGNU" signature virus. Just copy me to your
signature. This email was infected under the terms of the GNU

Keith Thompson
Guest
Posts: n/a

 03-01-2006
August Karlstrom <(E-Mail Removed)> writes:
> Xavier wrote:
>> August Karlstrom wrote:

> [snip]
>>> struct matrix
>>> {
>>> int rows, cols;
>>> double **at;
>>> };
>>> typedef struct matrix Matrix;

>> or all in one typedef struct matrix {...} Matrix;

>
> or just `typedef struct {} Matrix;'

Or just "struct matrix { ... };" and don't bother with the typedef.

> A style issue.

Indeed.

--
Keith Thompson (The_Other_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

Pedro Graca
Guest
Posts: n/a

 03-01-2006
Pedro Graca wrote:
> res->at = malloc(rows * sizeof *(res-at));

Oops ^^^^^^

I meant this instead
res->at = malloc(rows * sizeof *(res->at));

--