0

I'm currently learning c programming. I've written this code as part of an assignment and keep getting Segmentation faults. Prototypes:

#define ALLOC_ERROR -1

typedef struct monom
{
  int coefficient;    
  int power;        
} Monom;

int insertNewMonom(Monom** polynom, int base, int power, unsigned int size);
bool findPlace(Monom** polynom, int power, unsigned int size, int* place);
void printPolyMul(Monom* polynom1, unsigned int polynom1Size, Monom* polynom2, unsigned int polynom2Size);

Here is the relevant code:

void printPolyMul(Monom* polynom1, unsigned int polynom1Size, Monom* polynom2, unsigned int polynom2Size)
{
    Monom *mulPolynom = NULL, *temp = NULL;
    unsigned int  count = 0;
    int i, j, size=0, base, power;

    for(i=0; i<polynom1Size; i++){
      for(j=0; j<polynom2Size; j++){ 
        if(count==size){
            size = (size*2)+1;
            temp = realloc (sumPolynom, size * sizeof *sumPolynom);
            if(!temp){
                fprintf(stderr, "Allocation error\n");
                exit(ALLOC_ERROR);
            }
            else
                sumPolynom = temp;
        }                       
        base = (polynom1[i].coefficient)*(polynom2[j].coefficient);
        power = (polynom1[i].power)+(polynom2[j].power);
        count += insertNewMonom(&mulPolynom, base, power, count);
    }
}

temp =  realloc (mulPolynom, count * sizeof *mulPolynom);
if(!temp){
    fprintf(stderr, "Allocation Error\n");
    exit(ALLOC_ERROR);
}
else 
    mulPolynom = temp;

printPolynom(mulPolynom, count);            
free(mulPolynom);
}
int insertNewMonom(Monom** polynom, int base, int power, unsigned int size)
{
    int i, place;
    bool new_flag;
    Monom tempMonom;

    tempMonom.coefficient = base;
    tempMonom.power = power;    

    if(!base)
        return 0;
    new_flag = findPlace(polynom, power, size, &place);
    if(new_flag){
        if(place==size){
            (*polynom)[place] = tempMonom;
            return 1;
        }
        else{
            for(i=size; i>place; i--)
                (*polynom)[i] = (*polynom)[i-1];
            (*polynom)[place] = tempMonom;
            return 1;
        }
    }
    else{
        polynom[place]->coefficient += base;
        return 0;
    }   
}

bool findPlace(Monom** polynom, int power, unsigned int size, int* place)
{
    int curr;   
    for(curr=0; curr<size; curr++){
        if(polynom[curr]->power==power){
            *place = curr;
            return false;
        } 
        if(polynom[curr]->power<power){
            if(curr)        
                *place = curr-1;
            else
                *place = curr;
            return true;
        }
    }
    *place = size; 
    return true;
}

I managed to track down the exact point where the code crashes. It's when bool findPlace(Monom** polynom, int power, unsigned int size, int* place) is called for the third time, on the second for iteration (curr=1) when polynom[curr]->power is being checked. I'll mention that insertNewPolynom() was used earlier tin the program for receiving polynom1 and polynom2 and worked just fine.

AviadG
  • 45
  • 1
  • 6
  • 2
    Don't need to cast malloc in C http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Viktor Simkó Aug 10 '15 at 14:28
  • You can write: 'mulPolynom = realloc(mulPolynom, size * sizeof *mulPolynom)' – Viktor Simkó Aug 10 '15 at 14:31
  • @ViktorSimkó this leaks in case of error. – Quentin Aug 10 '15 at 14:57
  • @Quentin Then its better to write `mulPolynom = realloc(mulPolynom, size * sizeof(Monom))` ? – Viktor Simkó Aug 10 '15 at 15:57
  • 1
    @ViktorSimkó Thankes for the reply, i think i'ii keep casting until i'll have a better understanding of when its safe not to. any idea about why i get segmentation fault? – AviadG Aug 10 '15 at 15:59
  • 2
    @AGal *not* casting is **always** safe. It's what the type system is for. You should wonder when casting *is* safe. – Quentin Aug 10 '15 at 16:05
  • 1
    @ViktorSimkó no, you should store `realloc`'s return value in another variable, check if it succeeded, *then* overwrite`mulPolynom`. – Quentin Aug 10 '15 at 16:06
  • @Quentin Thanks, understood it now. Just haven't known what did You mean. – Viktor Simkó Aug 10 '15 at 16:15
  • @Quentin so `temp = realloc(mulPolynom, size * sizeof *mulPolynom); mulPolynom = temp;` is better? – AviadG Aug 10 '15 at 16:39
  • @AGal with an `if(!temp) { /* error, bailout */ }` in-between, yep – Quentin Aug 10 '15 at 16:42
  • @Quentin still getting that segmentation fault. any other ideas? – AviadG Aug 10 '15 at 17:10
  • BTW, the cast/ no cast issue and testing `realloc()` are not key to the problem. Fixing them will not solve the issue. OTOH, these 2 side issues, like spelling/grammar errors in a report, create distractions. So when code is not behaving as expected, it _is_ useful to attend to them so all may focus on the real issue. – chux - Reinstate Monica Aug 10 '15 at 17:35
  • 1) post `Monom` definition, 2) post `printPolynom(mulPolynom, count);` 3) are all warnings enabled? 4) Are functions prototyped somewhere before 1st use? They are not properly prototyped in the posted code. – chux - Reinstate Monica Aug 10 '15 at 17:54
  • @chux 1) posted, 2) its irellevant because the program doesn't get to that part, 3) yes, 4) posted the protorypes. – AviadG Aug 10 '15 at 18:17

2 Answers2

1

Size allocated is too small.

Consider size = polynom1Size*polynom2Size; realloc (mulPolynom, size * sizeof *mulPolynom); ... if(place==size){ (*polynom)[place] = tempMonom;.

Code is attempting to modify element [size] yet only has space for elements 0,1,2, ... size-1.

Without digging deep, it appears realloc (mulPolynom, (size + 1) * sizeof *mulPolynom); is needed.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • I checked that aspect, the logical size is always smaller than the pysical size... – AviadG Aug 10 '15 at 17:44
  • Suggest writing a function to use inside each `[foo(x, size)]` array access that test if `0<=x && x < size`. Code likely fails for 1) too small memory (this answer) 2) Array access out of bounds. (this comment). BTW: Did you try `realloc (mulPolynom, (size + 1) * sizeof *mulPolynom)`? – chux - Reinstate Monica Aug 10 '15 at 17:47
  • To be clear, this answer is not about a _logical size_ vs. a _physical size_ (bytes). It is about typical polynomial representation that is usually index `0,1,2,3, N` and so needs `N+1` elements. – chux - Reinstate Monica Aug 10 '15 at 17:58
  • That what i meant, the size is always at least N+1 – AviadG Aug 10 '15 at 18:05
0

In insertNewMonom you have statements

 (*polynom)[place] = tempMonom;

But tempMonom is a local variable and ceases to exist when the function returns. Any reference to it later on will either produce garbage or crash.

FredK
  • 4,094
  • 1
  • 9
  • 11
  • but when i write (*polynom)[place] = tempMonom i'm actually copying it, so later i wont have to reference to it, no? – AviadG Aug 10 '15 at 16:06