Gauss-Seidal implementation

The code is an implementation of the Gauss–Seidel method. I would like a general review of the code.

PS: I use code::blocks IDE

```
//generalized code for gauss seidal iteration method
#include
#include

#define chk_end 0.01

int row_lim=0,col_lim=0;

float partial_pivot(float mat,int row,int col);//finds the pivot of matrix
int condition(float result,float temp,int row);//checks the terminating condition
float final_result(float** mat,float result,int row,int col);//calculates final result
float create_mat(float mat,int row,int col);//creates a matrix of row X col
float feed_mat(float mat,int row,int col);//inputs value in matrix
void disp_mat(float** mat,int row,int col);//displays matrix

float partial_pivot(float mat,int row,int col)
{
float pivot;
int i=0,j=0,k=0,l=0;

if(row_lim temp[i])
{
if((result[i]-temp[i]) < chk_end)
{
flag = 1;
}
else
{
flag = 0;
break;
}
}

else
{
if((temp[i]-result[i]) < chk_end)
{
flag = 1;
}
else
{
flag = 0;
break;
}
}
}

return flag;
}

float final_result(float** mat,float result,int row,int col)
{
int k=0,i=0,j=0;
float T = 0;
float *temp;

temp=(float)malloc((row)sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
temp[i] = 0;
}

printf("Processing result ... \n");

do
{
for(k=0;k<row;k++)
temp[i] = result[i]; //was a logical error, sorry
{
for(i=0;i<row;i++)
{
for(j=0;j<(col-1);j++)
{
if(i==j)
{

}
else
{
T += (mat[i][j])*(temp[j]);
}
}
result[i] = ((mat[i][col-1] - T)/(mat[i][i]));
//temp[i] = result[i];
T=0;
}
}
}
while(condition(result,temp,row) == 0);

free(temp);

return result;
}

float create_mat(float mat,int row,int col)
{
int i=0;

mat=(float**)malloc((row)sizeof(float));//Edit: row-1 changed to row
if(mat)
{
for(i=0;i<row;i++)
{
mat[i]=(float)malloc((col)sizeof(float));//Edit

A word of warning: I am very much a C newbie myself, so it may be good to take my advice with a grain of salt.

Formatting

The only good thing I can say about your formatting is that it's consistent. But a 1-space indent really isn't readable. I recommend 4 spaces instead. Please put some whitespace around your operators: a space after every comma, a space on each side of =, -, +, *, …. Don't cram multiple assignments on one line: I find

int i, j, k;

or

int i = 0;
int j = 0;
int k = 0;

acceptable, but not int i=0,j=0,k=0;.

If you are targetting a C89-compatible environment, this is as far as we can go. I recommend you move on (e.g. to C++ or C99), where we can put the declaration of for-loop variables inside the loop itself:

for (int i = 0; i < whatever; i++)

… and we also don't have to put all declarations at the top of each scope – just declare your variables at the point of first use.

malloc and error handling

Always check the return value of malloc:

float *temp = (float*) malloc(row * sizeof(float));
if (temp == NULL)
{
    /* return some error code */
}

C isn't object oriented, but whatever

You always bundle the three variables mat, row, col. I suggest you put those into a struct, and pass this single struct around instead:

typedef struct {
    float **mat;
    int rows;
    int cols;
} Matrix;

I would then create a Matrix_new function that creates and populates an instance, e.g.

// this code assumes C99. Ignore my more compact brace style.

/**
 * Allocates a new `Matrix` instance. Returns a pointer to the instance on
 * success, a `NULL` pointer on failure.
 *
 * A Matrix must always be freed via `Matrix_destroy(instance)`!
 */

Matrix*
Matrix_new(int rows, int cols) {

    Matrix* matrix = (Matrix*) malloc(sizeof(Matrix));
    if (matrix == NULL) {
        return NULL;
    }

    float** mat = (float**) malloc(rows * sizeof(float*));
    if (mat == NULL) {
        free(matrix);
        return NULL;
    }

    for (int i = 0; i = 0; i--) {
                free(mat[i]);
            }
            free(mat);
            free(matrix);
            return NULL;
        }

    }

    matrix->mat  = mat;
    matrix->rows = rows;
    matrix->cols = cols;
    return matrix;
}

and of course, a mirroring Matrix_destroy:

/**
 * Safely frees a Matrix instance.
 */

int
Matrix_destroy(Matrix* matrix) {

    if (matrix == NULL) {
        return 0;
    }

    float** mat = matrix->mat;
    int rows = matrix->rows;

    for (int i = 0; i < rows; i++) {
        free(mat[i]);
    }

    free(mat);
    free(matrix);

    return 1;
}

Then, you can rewrite your other methods to take advantage of this bundling.

Return values

Very often (e.g. in final_result, create_mat, feed_mat), you return a pointer from your function even when that pointer was an argument to that function. It is generally preferable to return an error code, and to return values via pointers/out-arguments.

For example, final_result could be changed to:

/**
 * Calculates the final result.
 *
 * Returns an error code.
 *
 * After successful execution, the `out_result` pointer will point to the 
 * result, which is a `float*` of `self->rows` length. The caller owns the
 * result, and is responsible for calling `free(*out_result)`.
 *
 * Example usage:
 *
 *     float *result;
 *     if (! final_result(matrix, &result)) {
 *         // abort with error
 *     }
 *     // do something with `result`:
 *     for (int i = 0; i rows; i++) {
 *         printf("%f\n", result[i]);
 *     }
 *     free(result);
 */

int
final_result(Matrix* self, float** out_result) {
    if (self == NULL) {
        return 0;
    }

    float* temp = (float*) malloc(self->rows * sizeof(float));
    if (temp == NULL) {
        return 0;
    }

    float* result = (float*) malloc(self->rows * sizeof(float));
    if (result == NULL) {
        free(temp);
        return 0;
    }

    for (int i = 0; i rows; i++) {
        temp[i]   = 0;
        result[i] = 0;
    }

    printf("Processing result...\n");

    do {
        for (int i = 0; i rows; i++) {
            float T = 0;
            for (int j = 0; j cols - 1; j++) {
                if (i != j) {
                    T += self->mat[i][j] * temp[j];
                }
            }
            result[i] = (self->mat[i][self->cols - 1] - T) / self->mat[i][i];
            temp[i] = result[i];
        }
    } while (condition(result, temp, self->rows) == 0);

    free(temp);
    *out_result = result;
    return 1;
}

Miscellaneous Helper Functions

memset

When zeroing all elements in some data structure, you can often use memset from string.h instead:

temp=(float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
 temp[i] = 0;
}

becomes

``
float temp = (float) malloc(row * sizeof(float));
if (temp == NULL) {
// return error code
}
memset(temp, 0, row * sizeof(float));
`

StackExchange Code Review Q#45237, answer score: 6