Duration Conversion

[bcpkh]

[C application running on TRU64 Unix]

Hello All

I have a simple task that is driving me crazy. A string representing a
duration in the following format is passed to my application, a
function is dedicated to convert this duration to seconds;

[H]H:MM:SS

e.g. 0:00:00 or 00:12:45

The original function used atoi as part of the conversion process and
it produced some scary conversions 0:00:10 converted to 956 seconds
etc.

The function was rewritten to make use of strtol because of its better
error handling functionality but it now core dumps! Do someone have a
bullet proof idea to do this conversion assuming that anything might
be passed into this function;

e.g. A:00:00 or :0:45 etc

Reproducted below is the offending function the lines where it
currently dumps is marked with a @.

Any help would be much appreciated.

Thank you,

B

void secondDurations ()
{
long hr, min, sec, sec_duration = 0;
char tm[10];
int ch = ':';
char *end_ptr = NULL;
char duration[41];

char *pfirst;
char *plast;

// 00:00:00
// 0:00:00
// 01234567
strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) >= 7)
{
pfirst = strchr(duration , ch);
plast = strrchr(duratio n, ch);

//Test duration format 00:00:00
if((pfirst != NULL || plast != NULL) && (pfirst != plast))
{
errno = 0;

//Get hour portion
strcpy(tm, strtok(duration ,":"));

//Convert
hr = strtol(tm, &end_ptr, 10);

//Test
if (hr INT_MAX || hr < INT_MIN)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
errno = 0;

@ strcpy(tm, strtok(NULL,":" ));

min = strtol(tm, &end_ptr, 10);

if (min INT_MAX || min < INT_MIN || min 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
errno = 0;

@ strcpy(tm, strtok(NULL,":" ));

sec = strtol(tm, &end_ptr, 10);

if (sec INT_MAX || sec < INT_MIN || sec 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
sec_duration = hr * 3600 + min * 60 + sec;
printf("duratio n in seconds=%d\n",( int)sec_duratio n);
}
}
}
}
}

[Bart van Ingen Schenau]

bcpkh wrote:

[C application running on TRU64 Unix]

Hello All

I have a simple task that is driving me crazy. A string representing a
duration in the following format is passed to my application, a
function is dedicated to convert this duration to seconds;

[H]H:MM:SS

e.g. 0:00:00 or 00:12:45

The original function used atoi as part of the conversion process and
it produced some scary conversions 0:00:10 converted to 956 seconds
etc.

The function was rewritten to make use of strtol because of its better
error handling functionality but it now core dumps! Do someone have a
bullet proof idea to do this conversion assuming that anything might
be passed into this function;

e.g. A:00:00 or :0:45 etc

Reproducted below is the offending function the lines where it
currently dumps is marked with a @.

From reading your code, these invalid time strings should be rejected.
As the format is rather strict in what you can accept, I would use
sscanf() for this task.

void secondDurations ()
{
int hr, min, sec, num_matches, num_chars;
long sec_duration = 0;
char duration[41];

// 00:00:00
// 0:00:00
// 01234567

strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) < 7)
{
// Reject
return;
}

/* The trailing %n causes the number of characters consumed to be
recorded. This should be the entire length of the string */
num_matches = sscanf(duration , "%d:%d:%d%n ", &hr, &min, &sec,
&num_chars);
if ((num_matches != 3) || (num_chars != strlen(duration )))
{
// Malformed date. Reject
return;
}
else
{
sec_duration = hr * 3600 + min * 60 + sec;
printf("duratio n in seconds=%ld\n", sec_duration);
}
}

>
Any help would be much appreciated.

Thank you,

B

Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
c.l.c FAQ: http://www.eskimo.com/~scs/C-faq/top.html
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/

[Fred Kleinschmidt]

"bcpkh" <va************ **@gmail.comwro te in message
news:11******** **************@ 19g2000hsx.goog legroups.com...

[C application running on TRU64 Unix]

Hello All

I have a simple task that is driving me crazy. A string representing a
duration in the following format is passed to my application, a
function is dedicated to convert this duration to seconds;

[H]H:MM:SS

e.g. 0:00:00 or 00:12:45

The original function used atoi as part of the conversion process and
it produced some scary conversions 0:00:10 converted to 956 seconds
etc.

The function was rewritten to make use of strtol because of its better
error handling functionality but it now core dumps! Do someone have a
bullet proof idea to do this conversion assuming that anything might
be passed into this function;

e.g. A:00:00 or :0:45 etc

Reproducted below is the offending function the lines where it
currently dumps is marked with a @.

Any help would be much appreciated.

Thank you,

B

void secondDurations ()
{
long hr, min, sec, sec_duration = 0;
char tm[10];
int ch = ':';
char *end_ptr = NULL;
char duration[41];

char *pfirst;
char *plast;

// 00:00:00
// 0:00:00
// 01234567
strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) >= 7)
{
pfirst = strchr(duration , ch);
plast = strrchr(duratio n, ch);

//Test duration format 00:00:00
if((pfirst != NULL || plast != NULL) && (pfirst != plast))
{
errno = 0;

//Get hour portion
strcpy(tm, strtok(duration ,":"));

//Convert
hr = strtol(tm, &end_ptr, 10);

//Test
if (hr INT_MAX || hr < INT_MIN)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
errno = 0;

@ strcpy(tm, strtok(NULL,":" ));

min = strtol(tm, &end_ptr, 10);

if (min INT_MAX || min < INT_MIN || min 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
errno = 0;

@ strcpy(tm, strtok(NULL,":" ));

sec = strtol(tm, &end_ptr, 10);

if (sec INT_MAX || sec < INT_MIN || sec 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
sec_duration = hr * 3600 + min * 60 + sec;
printf("duratio n in seconds=%d\n",( int)sec_duratio n);
}
}
}
}
}

Try something like this:
long hrs, mins, sec;
char *ptr, *next;
hrs = strtol( duration, *ptr, 10 );
if ( (*ptr == ':' ) && (ptr != duration) ) {
/* hours read, try minutes */
next = ptr+1;
mins = strtol( next, &ptr, 10 );
if ( (*ptr == ':') && (ptr != next) } {
next = ptr+1;
sec = strtol( next, &ptr, 10 );
if ( *ptr == '\0' ) {
/* good - compute total seconds */

}
}
}

Other safety chacks should be made, such as that
ptr-duration is 1 or 2, and ptr-next is 2, etc.,
and that the hrs/mins/sec values are in the correct range.
--
Fred

[braam.vanheerden]

Hello Bart and Fred

Thank you for you suggestions and comments, you have been very
helpful! I will definitely make use of your inputs.

B

[Keith Thompson]

Bart van Ingen Schenau <ba**@ingen.ddn s.infowrites:

bcpkh wrote:

>[C application running on TRU64 Unix]
I have a simple task that is driving me crazy. A string representing a
duration in the following format is passed to my application, a
function is dedicated to convert this duration to seconds;

[H]H:MM:SS

e.g. 0:00:00 or 00:12:45

The original function used atoi as part of the conversion process and
it produced some scary conversions 0:00:10 converted to 956 seconds
etc.

[...]

From reading your code, these invalid time strings should be rejected.
As the format is rather strict in what you can accept, I would use
sscanf() for this task.

void secondDurations ()
{
int hr, min, sec, num_matches, num_chars;

[...]

/* The trailing %n causes the number of characters consumed to be
recorded. This should be the entire length of the string */
num_matches = sscanf(duration , "%d:%d:%d%n ", &hr, &min, &sec,
&num_chars);

[...]

If the string you're scanning contains a syntactically valid integer
value that can't be stored in an int, then sscanf with the "%d" format
invokes undefined behavior. This is true for all the numeric
conversion formats for the *scanf() functions.

If you want safety, you'll need to check for the number of digits
before invoking sscanf, or you'll need to use some other method (like
strtol).

I haven't studied the original code, so I don't know why it's blowing
up.

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Old Wolf]

On Sep 14, 3:25 am, bcpkh <vanheerden.br. [e-mail] te:

[C application running on TRU64 Unix]
The original function used atoi as part of the conversion process and
it produced some scary conversions 0:00:10 converted to 956 seconds
etc.

The function was rewritten to make use of strtol because of its better
error handling functionality but it now core dumps! Do someone have a
bullet proof idea to do this conversion assuming that anything might
be passed into this function;

You forgot the following lines:
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <limits.h>

Once those were included, and an output line and a main function
added, the code compiles and runs fine for me.

You should have gotten many warnings when you compiled
the code -- if not, then find out how to turn up the
warning level on your compiler.

strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) >= 7)
{

You have a weird code structure here. It goes:
if (A)
{
if (B)
return;
else if (C)
return;
else
{
if (D)
return;
else if (E)
return;
else
{
/* dozens of lines */
printf(......);
}
}
/* end of function */

It is hard to follow. IMHO, much clearer is:
if ( !A )
return;
if ( B )
return;
if ( C )
return;
if ( D )
return;
if ( E )
return;
/* dozens of lines */
/* print results */

//Get hour portion
strcpy(tm, strtok(duration ,":"));

strtok would return NULL if the stirng contains no colon.
As posted, this can't happen, but what if somebody changes
the 'ch' variable but does not update the strtok calls?
You should use the same variable for both.

Also, if the duration string contains a lot of characters
between the colons, you cause a buffer overflow when copying
into tm.

What is the purpose of this "tm" ? You could have
written:
hr = strtol( strtok(NULL, ":"), &end_ptr, 10 );

although in either case I would prefer that the result
of strtok be stored in an intermediate variable and
checked that it is not NULL.

[Barry Schwarz]

On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <va************ **@gmail.com>
wrote:
snip explanation

>void secondDurations ()
{
long hr, min, sec, sec_duration = 0;
char tm[10];
int ch = ':';
char *end_ptr = NULL;
char duration[41];

char *pfirst;
char *plast;

// 00:00:00
// 0:00:00
// 01234567
strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) >= 7)
{
pfirst = strchr(duration , ch);
plast = strrchr(duratio n, ch);

Is it possible for pfirst to be different from plast? Can these two
statements ever produce different results?

>
//Test duration format 00:00:00
if((pfirst != NULL || plast != NULL) && (pfirst != plast))
{
errno = 0;

//Get hour portion
strcpy(tm, strtok(duration ,":"));

//Convert
hr = strtol(tm, &end_ptr, 10);

//Test
if (hr INT_MAX || hr < INT_MIN)

On some systems, sizeof(long) is the same as sizeof(int).

{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject

You set errno to 0. Did you intend to set it to some other value now?

return;
}
else
{
errno = 0;

@ strcpy(tm, strtok(NULL,":" ));

This will allow a string of the form 01::23:45 to be treated as
correct.

>
min = strtol(tm, &end_ptr, 10);

if (min INT_MAX || min < INT_MIN || min 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
errno = 0;

You only need to set it to 0 once. It can't become any more zero than
it is.

>
@ strcpy(tm, strtok(NULL,":" ));

sec = strtol(tm, &end_ptr, 10);

if (sec INT_MAX || sec < INT_MIN || sec 59)
{
//Reject;
return;
}
else if (end_ptr == tm || *end_ptr != '\0')
{
//Reject
return;
}
else
{
sec_duration = hr * 3600 + min * 60 + sec;
printf("duratio n in seconds=%d\n",( int)sec_duratio n);
}
}
}
}
}

Remove del for email

[Barry Schwarz]

On Thu, 13 Sep 2007 17:23:32 -0700, Barry Schwarz <sc******@doezl .net>
wrote:

>On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <va************ **@gmail.com>
wrote:
snip explanation

>>void secondDurations ()
{
long hr, min, sec, sec_duration = 0;
char tm[10];
int ch = ':';
char *end_ptr = NULL;
char duration[41];

char *pfirst;
char *plast;

// 00:00:00
// 0:00:00
// 01234567
strcpy(duration , "00:00:34") ;

//String should at leat be 7 characters long
if (strlen(duratio n) >= 7)
{
pfirst = strchr(duration , ch);
plast = strrchr(duratio n, ch);

Is it possible for pfirst to be different from plast? Can these two
statements ever produce different results?

Obviously they can if I would only notice the fourth r in the second
statement.
Remove del for email

[CBFalconer]

Old Wolf wrote:

>

.... snip ...

>
It is hard to follow. IMHO, much clearer is:
if ( !A )
return;
if ( B )
return;
if ( C )
return;
if ( D )
return;
if ( E )
return;
/* dozens of lines */
/* print results */

I would prefer:

if (!(!A || B || C || D || E)) {
/* dozens of lines */
/* print results */
}
return;

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net>

--
Posted via a free Usenet account from http://www.teranews.com

[jaysome]

On Thu, 13 Sep 2007 23:24:33 -0400, CBFalconer <cb********@yah oo.com>
wrote:

>Old Wolf wrote:

>>

... snip ...

>>
It is hard to follow. IMHO, much clearer is:
if ( !A )
return;
if ( B )
return;
if ( C )
return;
if ( D )
return;
if ( E )
return;
/* dozens of lines */
/* print results */

I would prefer:

if (!(!A || B || C || D || E)) {
/* dozens of lines */
/* print results */
}
return;

Applying De Morgan's Theorem to your expression I get:

if ( A && !B && !C && !D && !E )
{
/* dozens of lines */
/* print results */
}

This is clearer to me, and it ostensibly takes advantage of C's
short-circuit evaluation of conditions to determine the value of the
expression. It's unclear to me whether your expression takes advantage
of C's short-circuit evaluation of conditions, given that everything
in the inner-most parenthesis is surrounded by a '!'.

Whether it is clearer to you or anyone else is a subjective matter.
Nevertheless, it's useful to apply De Morgan's Theorem to any
non-trivial expression as it gives you a pair of alternatives that
often makes it clear which one is the best, in terms of readability
and possibly even performance.

--
jay