LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (https://www.linuxquestions.org/questions/programming-9/)
-   -   program taken out of book won't run (https://www.linuxquestions.org/questions/programming-9/program-taken-out-of-book-wont-run-4175516538/)

doughyi8u 08-27-2014 10:23 AM
program taken out of book won't run
 
The following code was taken out of a book but gets a seg fault:
Code:
//time.h
#include <cstdlib>
#include <stdio.h>

struct time {
    long secs;

    void set_time(char *);
    void get_time(char *);
};

void time::set_time(char *tod24)
{
    long hours, minutes;

    minutes=atol(&tod24[2]);//this line doesn't make sence to me
    tod24[2] =  '\0';neither does this
    hours = atol(tod24);
    secs = (hours*3600)+(minutes*60);
}

void time::get_time(char *tod)
{
    static char *tday[] = {"MIDNIGHT","P.M.","NOON","A.M."};
    int hours, minutes, amflag = 0;

    hours = secs/3600;
    minutes = (secs % 3600)/60;

    if (hours == 24) {
        hours = 0;
        amflag = (minutes == 0) ? 0 : 3;
    } else if (hours > 12) {
        hours = hours - 12;
        amflag = 1;
    } else if (hours == 12)
    amflag = 2;
    else
        amflag = 3;

        sprintf(tod,"%2d:%02d %s",hours,minutes,tday[amflag]);

}

timetest.c
#include "time.h"
#include <cstdlib>
#include <stdio.h>

int
main()
{
    char tstring[40];
    time x;

    x.secs = 23;
    x.set_time("2000");
    x.get_time(tstring);
    puts(tstring);

    return 0;
}
The comment in red above confuses me. Like I said, this is taken directly out of a book. Could someone explain how the comment in red above works or if I'm not alone on seeing it as an error

NevemTeve 08-27-2014 11:00 AM
Try this:

Code:
void time::set_time (const char hhmm[4])
{
    char tmp[3];
    long hours, minutes;

    memcpy (tmp, hhmm, 2);
    tmp[2]= '\0';
    hours= atoi (tmp);

    memcpy (tmp, hhmm+2, 2);
    tmp[2]= '\0';
    minutes= atoi (tmp);

    this->secs= (hours + 60*minutes)*60;
}

gdejonge 08-27-2014 11:06 AM
Hi,

&tod24[2] means take address of the 3th character of the string and give that to the atol function.

'\0' is the string terminator, so you cut off the string at the 3th character.

mina86 08-27-2014 12:27 PM
“&tod24[2]” is equivalent to “tod24 + 2” (but some people like the awkward form ;) ). If this still does not make sense to you, try this code:
Code:
#include <stdio.h>

int main(void) {
    const char *str = "1234";
    puts(str);
    puts(str + 1);
    puts(str + 2);
    return 0;
}

doughyi8u 08-27-2014 12:33 PM
I understand the programming in this (what it does) but why would I want to do that in this program? Why is it giving me a seg fault?

NevemTeve 08-27-2014 12:47 PM
Cause it was written by an amateur. And back then the compilers were less strict.

mina86 08-27-2014 12:57 PM
Quote:
Originally Posted by doughyi8u (Post 5228194)
I understand the programming in this (what it does) but why would I want to do that in this program?
Consider this:
Code:
#include <stdio.h>

int main(void) {
  static char time[] = "2000";
  printf("minutes = %s\n", time + 2);
  time[2] = 0;
  printf("hours = %s\n", time);
  return 0;
}
Quote:
Originally Posted by doughyi8u (Post 5228194)
Why is it giving me a seg fault?
Because the fact that string literal can be assigned to a pointer to non-const char is historical legacy. Consider this instead:
Code:
int main() {
    static const char twohundredhours[] = "2000";

    char tstring[40];
    time x;

    x.secs = 23;
    x.set_time(twohundredhours);
    x.get_time(tstring);
    puts(tstring);

    return 0;
}
Notice how it does not compile even though it's, in some sense, the very same code.

PS. And yeah, similarly to what NevemTeve has said, based on the code, I would recommend you stop reading this book and use it for something useful like starting a fire place.

doughyi8u 08-27-2014 02:36 PM
Quote:
PS. And yeah, similarly to what NevemTeve has said, based on the code, I would recommend you stop reading this book and use it for something useful like starting a fire place.
LOL

Any comprehensive book suggestions on c++?

dugan 08-27-2014 02:39 PM
Quote:
Originally Posted by doughyi8u (Post 5228263)
Any comprehensive book suggestions on c++?
https://www.informit.com/store/c-plu...-9780321714114

jpollard 08-31-2014 12:52 PM
Quote:
Originally Posted by doughyi8u (Post 5228194)
I understand the programming in this (what it does) but why would I want to do that in this program? Why is it giving me a seg fault?
It depends on the input...

If you provided h:m format (where h is a digit for hours, and m one or more digits for minutes), starting on the third character would convert the minutes. The second part converts the hours, but assumes that the conversion will terminate when it reaches a non digit character (the ":" in my example format).

Unfortunately, it won't work correctly if you provide "h" or "hh" where "h" is a digit of the hours. This is because the third character would be at or beyond the end of the string - and hopefully only causes a segfault, because you are accessing data beyond the end of a buffer.

Looking at the entire program, I suspect the book is fairly old. It should be using snprintf to prevent formatting past the buffer, and the parameter should include the buffer length...

bulliver 09-03-2014 04:50 PM
Quote:
Originally Posted by dugan (Post 5228265)
Yes, I have C++ Primer Plus, it is pretty decent. Bjarne's own "The C++ Programming Language" is also good, but very dense and probably not best for a beginner.

psionl0 09-04-2014 01:23 AM
Quote:
Originally Posted by doughyi8u (Post 5228194)
I understand the programming in this (what it does) but why would I want to do that in this program? Why is it giving me a seg fault?
The method void time::set_time(char *tod24) modifies the character array tod24[] yet it is called with a string literal x.set_time("2000");. If that literal is rodata then it would cause a seg fault.

One way to avoid this is for set_time() to have its own character array.
eg:
Code:
void time::set_time(char *tod24)
{
    long hours, minutes;
    char hourString[4];

    minutes=atol(&tod24[2]);//this line doesn't make sence to me
/*    tod24[2] =  '\0';neither does this
    hours = atol(tod24);                  */
    hours = atol(strncpy(hourString, tod24, 2));
    secs = (hours*3600)+(minutes*60);
}

mina86 09-04-2014 02:46 AM
Quote:
Originally Posted by psionl0 (Post 5232157)
One way to avoid this is for set_time() to have its own character array.
At which point you also want to change the prototype of the method to:
Code:
void time::set_time(const char *tod24);


All times are GMT -5. The time now is 08:12 PM.