Help with segmentation fault in struct

Question

Hello. I am having trouble debugging my code. I have a struct and a function to compute the time difference entered in HH:MM:SS format. My code is:

const int hourConv = 3600; // used to get total hours from total seconds 
const int minConv = 60; 
struct MyTime { 
    int hours, minutes, seconds; 
}; 

MyTime *determineElapsedTime(const MyTime *time1, const MyTime *time2) 
{ 
        long timeOneSec = time1->hours*hourConv + time1->minutes*minConv + time1->seconds; 
        long timeTwoSec = time2->hours*hourConv + time2->minutes*minConv + time2->seconds; 
        long ans = timeTwoSec - timeOneSec; 
        cout << ans; 
        MyTime *timeDiff; 
        timeDiff->hours = ans / hourConv; 
        timeDiff->minutes = ans % hourConv / minConv; 
        timeDiff->seconds = ans % hourConv % minConv; 
        return timeDiff; 
}

I believe the problem to be with the 2nd to last line: timeDiff->seconds = ans%hourConv%minConv; since when i comment that line out, I do not get a segmentation fault error. But I don't understand why that line is invalid. Any help would be appreciated. Thanks!

Answer 1

Your code contains:

MyTime *timeDiff;
timDiff->hours = ...

You have created a MyTime pointer but not allocated anything. timeDiff is null at this point.

Answer 2

You're trying to access unallocated memory with the following code:

MyTime *timeDiff;
timeDiff->hours = ans / hourConv;

Although you could solve this by manually allocating the code using new, as:

MyTime *timeDiff = new MyTime;
timeDiff->hours = ans / hourConv;

I'd highly recommend changing your function to return the MyStruct by value, as a stack-allocated variable. I'd also suggest taking the arguments as pass-by-const reference:

MyTime determineElapsedTime(MyTime const &time1, MyTime const &time2)
{
     long timeOneSec = time1.hours*hourConv + time1.minutes*minConv + time1.seconds;
     long timeTwoSec = time2.hours*hourConv + time2.minutes*minConv + time2.seconds;
     long ans = timeTwoSec - timeOneSec;
     cout << ans;
     MyTime timeDiff;
     timeDiff.hours = ans / hourConv;
     timeDiff.minutes = ans % hourConv / minConv;
     timeDiff.seconds = ans % hourConv % minConv;
     return timeDiff;
}

Answer 3

Just one other remark: use OOP in this case. It will make your code so much more readable. You'll end up with more time to think about uninitialized memory.

struct MyTime { 
    int hours, minutes, seconds; 
    int timeInSeconds() const {
        return hours*3600 + minutes*60 + seconds;
    }
    // the difference, in seconds
    int operator-( const MyTime other ) const {
        return timeInSeconds() - other.timeInSeconds();
    }
    void subtract( int seconds ) {
        seconds -= seconds;
        while( seconds < 0 ) { seconds += 60; --minutes; }
        while( minutes < 0 ) { minutes += 60; --hours; }
        assert( hours >= 0 );
    }
};

Next to that, it's a good idea to differentiate between time-differences and 'absolute' time values. You can add two time diffs, but you cannot add two 'calendar' values.