Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conman fails when timezone is greater than 4 characters. #16

Closed
GoogleCodeExporter opened this issue Mar 17, 2015 · 7 comments
Closed
Labels
Milestone

Comments

@GoogleCodeExporter
Copy link

What steps will reproduce the problem?

/usr/share/zoneinfo/Etc/GMT-1 used for /etc/localtime

What is the expected output? What do you see instead?

expected conman deamon to function, instead saw the log message;

NOTICE: Starting ConMan daemon 0.2.5 (pid 86513)
INFO: Open file limit set to 1024
ERROR: strftime() failed

NB: same problem with 0.2.7

What version of the software are you using? On what operating system?

conman 0.2.5-2.4.el6 as shipped with SL6.3. Noticed same problem in source tip of tree.

Please provide any additional information below.

The problem is create_long_time_string(time_t t) has const int len set to 25, when it should be 29 (or higher) to deal with all known current timezone codes. Recommend it be set to 32 to provide an engineering margin.

in util_str.c:

char * create_long_time_string(time_t t)
{
    char *p;
    struct tm tm;
    const int len = 25;                 /* YYYY-MM-DD HH:MM:SS ZONE + NUL */

    if (!(p = malloc(len))) {
        out_of_memory();
    }
    get_localtime(&t, &tm);

    if (strftime(p, len, "%Y-%m-%d %H:%M:%S %Z", &tm) == 0) {
        log_err(0, "strftime() failed");
    }
    return(p);
}

Original issue reported on code.google.com by mark_salyzyn@xyratex.com on 3 Jan 2013 at 8:54

@GoogleCodeExporter
Copy link
Author

Original comment by chris.m.dunlap on 3 Jan 2013 at 8:58

  • Changed state: Accepted
  • Added labels: Milestone-0.2.8

@GoogleCodeExporter
Copy link
Author

I thought every time zone could be abbreviated with 4 characters or less:

http://www.timeanddate.com/library/abbreviations/timezones/

What does strftime() return for your %Z value?

Original comment by chris.m.dunlap on 3 Jan 2013 at 9:35

@GoogleCodeExporter
Copy link
Author

"GMT-1"

We instructed out customer to use Europe/Vienna which works and is the correct zone as the workaround.

Also tried with Europe/Volgograd and it reported "VOLT-4", America/Argentina/San_Luis "WARST" & Chile/EasterIsland "EASST". I searched the zoneinfo database and found some as long as 8 characters.

Original comment by mark_salyzyn@xyratex.com on 3 Jan 2013 at 9:53

@GoogleCodeExporter
Copy link
Author

This issue was closed by revision 858ecb4.

Original comment by chris.m.dunlap on 3 Jan 2013 at 10:41

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

You are trying to minimize buffer size because you don't want to waste bytes, right?

There is a better way to do it:

char * create_long_time_string(time_t t)
{
    char buf[160];
    struct tm tm;

    get_localtime(&t, &tm);

    if (strftime(buf, sizeof(buf), "%Y-%m-%d %H:%M:%S %Z", &tm) == 0) {
        log_err(0, "strftime() failed");
        buf[0] = '\0';
    }
    return(create_string(buf));
}

This way, you always allocate as many bytes as necessary, and not a byte more.

Original comment by vda.linux on 20 Feb 2013 at 12:25

@GoogleCodeExporter
Copy link
Author

True, but that costs an additional strcpy (ie, a strdup vs a malloc).

If there was a greater range in the lengths of timezone strings (eg, if there was at least one tz string that was really long), I'd be inclined to go with this approach. But we're only dealing with a few extra bytes for a short-lived string. Thanks, though.

Original comment by chris.m.dunlap on 21 Feb 2013 at 12:03

@GoogleCodeExporter
Copy link
Author

that costs an additional strcpy (ie, a strdup vs a malloc).

The cost is small, while wasted trailing bytes will persist as long as result is in use.

Original comment by vda.linux on 7 May 2013 at 2:00

@dun dun added bug and removed auto-migrated labels May 16, 2015
@dun dun added this to the 0.2.8 milestone May 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants