Help with segmentation fault in struct

Question

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!

@john you were a newbie at one point too... give the guy a break, it's not complex code. — Jim Garrison
– Jim Garrison, Commented Nov 10, 2009 at 5:11

Jim Garrison · Accepted Answer · 2009-11-10 05:10:38Z

6

Your code contains:

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

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

answered Nov 10, 2009 at 5:10

Jim Garrison

87k20 gold badges162 silver badges197 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

bbg Over a year ago

Is it null or a garbage value?

Jim Garrison Over a year ago

I believe in C/C++ the value is undefined at that point (correct me if I'm wrong)

Crystal Over a year ago

Ok that makes sense about it being null. But what would I allocate it to at this point since I'm trying to create a new struct and a return a pointer to that struct? Thanks!

Crystal Over a year ago

Or are you supposed to say: MyTime timeDiff timeDiff.hours = ... return &timeDiff; although this way I get a compiler warning about returning the address of MyTime. Thanks!

Jim Garrison Over a year ago

You have to allocate a new instance of MyTime and assign it to timeDiff. Then you can fill in the values and return the pointer to the allocated structure. Remeber that the caller will need to free the struct or you will create a memory leak.

|

Jon Benedicto · Accepted Answer · 2009-11-10 06:05:00Z

5

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;
}

edited Nov 10, 2009 at 6:05

answered Nov 10, 2009 at 5:42

Jon Benedicto

10.6k3 gold badges31 silver badges31 bronze badges

1 Comment

Jonathan Leffler Over a year ago

The % hourConv is of no value in the assignment to timeDiff.seconds. It will be hard for the compiler to spot that.

xtofl · Accepted Answer · 2009-11-10 06:06:57Z

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.

Collectives™ on Stack Overflow

Help with segmentation fault in struct

3 Answers 3

6 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related