conman fails when timezone is greater than 4 characters.

[GoogleCodeExporter]

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:
{code}
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);
}
{code}

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

[GoogleCodeExporter]

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

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

[GoogleCodeExporter]

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]

"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_sal...@xyratex.com on 3 Jan 2013 at 9:53

[GoogleCodeExporter]

This issue was closed by revision 858ecb47c558.

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

• Changed state: Fixed

[GoogleCodeExporter]

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.li...@googlemail.com on 20 Feb 2013 at 12:25

[GoogleCodeExporter]

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]

> 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.li...@googlemail.com on 7 May 2013 at 2:00