Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Duration Conversion

Reply
Thread Tools

Duration Conversion

 
 
bcpkh
Guest
Posts: n/a
 
      09-13-2007
[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(duration) >= 7)
{
pfirst = strchr(duration, ch);
plast = strrchr(duration, 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("duration in seconds=%d\n",(int)sec_duration);
}
}
}
}
}

 
Reply With Quote
 
 
 
 
Bart van Ingen Schenau
Guest
Posts: n/a
 
      09-13-2007
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(duration) < 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("duration 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/
 
Reply With Quote
 
 
 
 
Fred Kleinschmidt
Guest
Posts: n/a
 
      09-13-2007

"bcpkh" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed) ups.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(duration) >= 7)
> {
> pfirst = strchr(duration, ch);
> plast = strrchr(duration, 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("duration in seconds=%d\n",(int)sec_duration);
> }
> }
> }
> }
> }
>


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


 
Reply With Quote
 
braam.vanheerden@gmail.com
Guest
Posts: n/a
 
      09-13-2007
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

 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      09-13-2007
Bart van Ingen Schenau <(E-Mail Removed)> writes:
> 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_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <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"
 
Reply With Quote
 
Old Wolf
Guest
Posts: n/a
 
      09-13-2007
On Sep 14, 3:25 am, bcpkh <(E-Mail Removed)> wrote:
> [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(duration) >= 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.


 
Reply With Quote
 
Barry Schwarz
Guest
Posts: n/a
 
      09-14-2007
On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <(E-Mail Removed)>
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(duration) >= 7)
> {
> pfirst = strchr(duration, ch);
> plast = strrchr(duration, 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("duration in seconds=%d\n",(int)sec_duration);
> }
> }
> }
> }
> }



Remove del for email
 
Reply With Quote
 
Barry Schwarz
Guest
Posts: n/a
 
      09-14-2007
On Thu, 13 Sep 2007 17:23:32 -0700, Barry Schwarz <(E-Mail Removed)>
wrote:

>On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <(E-Mail Removed)>
>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(duration) >= 7)
>> {
>> pfirst = strchr(duration, ch);
>> plast = strrchr(duration, 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
 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      09-14-2007
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

 
Reply With Quote
 
jaysome
Guest
Posts: n/a
 
      09-14-2007
On Thu, 13 Sep 2007 23:24:33 -0400, CBFalconer <(E-Mail Removed)>
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
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Call Manager limit call duration nazgulero Cisco 0 10-25-2005 06:58 AM
What is the Time Duration of all MCSD.net Exams? IMRAN SAROIA MCSD 0 04-04-2004 08:31 PM
Exam duration Girish Hegde MCSD 1 01-13-2004 02:47 AM
PIX 501 - SYN Timeout Duration 0:02:01 Rik Bain Cisco 11 11-03-2003 11:50 PM
Work Duration sunil chandra MCSD 2 08-28-2003 04:15 PM



Advertisments