Velocity Reviews - Computer Hardware Reviews

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

Reply
Thread Tools

Duration Conversion

 
 
Richard Bos
Guest
Posts: n/a
 
      09-14-2007
Keith Thompson <(E-Mail Removed)> wrote:

> Bart van Ingen Schenau <(E-Mail Removed)> writes:
> > bcpkh wrote:
> >> 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


> > 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).


That's why his function carefully checks that the length of the _whole_
string is less than 7. Since INT_MAX can be 32767, that still isn't
careful enough, but it's some way towards the right amount of care.
Using longs instead of ints would solve the problem.

Richard
 
Reply With Quote
 
 
 
 
Charlie Gordon
Guest
Posts: n/a
 
      09-14-2007
"Old Wolf" <(E-Mail Removed)> a écrit dans le message de news:
(E-Mail Removed) om...
> On Sep 14, 3:25 am, bcpkh <(E-Mail Removed)> wrote:

[...]
>> //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.


Do not use strtok: it is non reentrant. It used a private global state
variable. It is error prone even in single threaded programs. A reentrant
version strtok_r is available on POSIX systems, but you hardly need
something to heavy for a simple parsing task such as this one. Here is a
program that performs the needed conversion, feel free to copy the relevant
function.

#include <stdio.h>
#include <ctype.h>

/* Convert a timestamp to a number of seconds:
* format is H:MM:SS or HH:MM:SS
* hour must be in range [0..23]
* minutes and seconds must be in range [0..59]
* return -1 in case of NULL pointer or invalid format
*/
long convert_time(const char *str) {
int hour, min, sec;

if (!str || !isdigit(*str))
return -1;
hour = *str++ - '0';
if (isdigit(*str))
hour = hour * 10 + (*str++ - '0');
if (*str != ':' || !isdigit(str[1]) || !isdigit(str[2]))
return -1;
min = (str[1] - '0') * 10 + (str[2] - '0');
str += 3;
if (*str != ':' || !isdigit(str[1]) || !isdigit(str[2]))
return -1;
sec = (str[1] - '0') * 10 + (str[2] - '0');
str += 3;
/* here you can relax the check on what can follow the time stamp */
if (*str != '\0')
return -1;
/* here you can customize the checks on timestamp validity */
if (hour > 23 || min > 59 || sec > 59)
return -1;
return hour * 3600L + min * 60 + sec;
}

int main(int argc, char **argv) {
int i;
for (i = 1; i < argc; i++) {
printf("\"%s\" -> %ld\n", argv[i], convert_time(argv[i]));
}
return 0;
}


--
Chqrlie.


 
Reply With Quote
 
 
 
 
Richard
Guest
Posts: n/a
 
      09-14-2007
CBFalconer <(E-Mail Removed)> writes:

> 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;


much clearer is

if (B || ... || (!A))
return

dozens of lines.

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

 
Reply With Quote
 
Richard Bos
Guest
Posts: n/a
 
      09-14-2007
Richard <(E-Mail Removed)> wrote:

> CBFalconer <(E-Mail Removed)> writes:
>
> > I would prefer:
> >
> > if (!(!A || B || C || D || E)) {
> > /* dozens of lines */
> > /* print results */
> > }
> > return;

>
> much clearer is
>
> if (B || ... || (!A))
> return


That may be, but it doesn't have the same semantics, because || is a
short-circuiting operator.

Richard
 
Reply With Quote
 
Richard
Guest
Posts: n/a
 
      09-14-2007
(E-Mail Removed) (Richard Bos) writes:

> Richard <(E-Mail Removed)> wrote:
>
>> CBFalconer <(E-Mail Removed)> writes:
>>
>> > I would prefer:
>> >
>> > if (!(!A || B || C || D || E)) {
>> > /* dozens of lines */
>> > /* print results */
>> > }
>> > return;

>>
>> much clearer is
>>
>> if (B || ... || (!A))
>> return

>
> That may be, but it doesn't have the same semantics, because || is a
> short-circuiting operator.
>
> Richard


True. I neglected to consider that A,B etc could be expressions/function
calls. restore the order.

The main point is I like the early exit. I always find that more
intuitive when debugging and not having to jump over code or let my eyes
roam to find the return.

IOW, if I shouldn't be there, get out.
 
Reply With Quote
 
Richard Bos
Guest
Posts: n/a
 
      09-14-2007
Richard <(E-Mail Removed)> wrote:

> (E-Mail Removed) (Richard Bos) writes:
>
> > Richard <(E-Mail Removed)> wrote:
> >
> >> CBFalconer <(E-Mail Removed)> writes:
> >>
> >> > I would prefer:
> >> >
> >> > if (!(!A || B || C || D || E)) {
> >> > /* dozens of lines */
> >> > /* print results */
> >> > }
> >> > return;
> >>
> >> much clearer is
> >>
> >> if (B || ... || (!A))
> >> return

> >
> > That may be, but it doesn't have the same semantics, because || is a
> > short-circuiting operator.

>
> True. I neglected to consider that A,B etc could be expressions/function
> calls. restore the order.
>
> The main point is I like the early exit. I always find that more
> intuitive when debugging and not having to jump over code or let my eyes
> roam to find the return.


That, to me, depends on whether A, B, C and so on are true error
conditions or normal logic flow decisions. I tend to bail out early for
things like "we got passed a null pointer" and late for "this customer's
deduction works out to zero".

Richard
 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      09-14-2007
jaysome wrote:
> CBFalconer <(E-Mail Removed)> wrote:
>

.... snip long coded sample ...
>
>> 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 '!'.


While true enough, also consider the effort of evaluating the
conditional (although many optimizers will eliminate the
difference). With my statement, the decision to print is reached
after chacking a minimum number of components, while your 'reduced'
version requires all components to be checked. Of course the
reverse applies if optimizing for the 'skip it all' case.

(and yes, the short circuit operation functions in both)

At any rate my point was to eliminate the long confusing barrage of
lines in the original.

--
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
 
Ben Bacarisse
Guest
Posts: n/a
 
      09-14-2007
Keith Thompson <(E-Mail Removed)> writes:

> 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).


There is another option. One can limit the number of digits read.
Also, the OP can use a trick to ensure the last number is no more than
two digits long:

sscanf(duration, "%2d:%2d:%2d%c", &hr, &min, &sec, &end) == 3

means the data is valid. The test is for three not four conversions
because the final one failing indicates that the string ends after no
more that a two-digit number. I think this is safe from undefined
behaviour.

If the string might not end after the last digit one can do:

nc = sscanf(duration, "%2d:%2d:%2d%c", &hr, &min, &sec, %end);
if (nc == 3 || nc == 4 && !isdigit(end)) /* data OK */

(I think 'unsigned char end;' is the best type for this kind of thing.)

--
Ben.
 
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