Duration Conversion

Discussion in 'C Programming' started by bcpkh, Sep 13, 2007.

  1. bcpkh

    bcpkh Guest

    [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);
    }
    }
    }
    }
    }
     
    bcpkh, Sep 13, 2007
    #1
    1. Advertising

  2. 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/
     
    Bart van Ingen Schenau, Sep 13, 2007
    #2
    1. Advertising

  3. "bcpkh" <> wrote in message
    news:...
    > [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
     
    Fred Kleinschmidt, Sep 13, 2007
    #3
  4. bcpkh

    Guest

    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
     
    , Sep 13, 2007
    #4
  5. Bart van Ingen Schenau <> 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.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"
     
    Keith Thompson, Sep 13, 2007
    #5
  6. bcpkh

    Old Wolf Guest

    On Sep 14, 3:25 am, bcpkh <> 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.
     
    Old Wolf, Sep 14, 2007
    #6
  7. On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <>
    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
     
    Barry Schwarz, Sep 14, 2007
    #7
  8. On Thu, 13 Sep 2007 17:23:32 -0700, Barry Schwarz <>
    wrote:

    >On Thu, 13 Sep 2007 15:25:03 -0000, bcpkh <>
    >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
     
    Barry Schwarz, Sep 14, 2007
    #8
  9. bcpkh

    CBFalconer Guest

    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
     
    CBFalconer, Sep 14, 2007
    #9
  10. bcpkh

    jaysome Guest

    On Thu, 13 Sep 2007 23:24:33 -0400, CBFalconer <>
    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
     
    jaysome, Sep 14, 2007
    #10
  11. bcpkh

    Richard Bos Guest

    Keith Thompson <> wrote:

    > Bart van Ingen Schenau <> 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
     
    Richard Bos, Sep 14, 2007
    #11
  12. "Old Wolf" <> a écrit dans le message de news:
    ...
    > On Sep 14, 3:25 am, bcpkh <> 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, convert_time(argv));
    }
    return 0;
    }


    --
    Chqrlie.
     
    Charlie Gordon, Sep 14, 2007
    #12
  13. bcpkh

    Richard Guest

    CBFalconer <> 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>
     
    Richard, Sep 14, 2007
    #13
  14. bcpkh

    Richard Bos Guest

    Richard <> wrote:

    > CBFalconer <> 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
     
    Richard Bos, Sep 14, 2007
    #14
  15. bcpkh

    Richard Guest

    (Richard Bos) writes:

    > Richard <> wrote:
    >
    >> CBFalconer <> 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.
     
    Richard, Sep 14, 2007
    #15
  16. bcpkh

    Richard Bos Guest

    Richard <> wrote:

    > (Richard Bos) writes:
    >
    > > Richard <> wrote:
    > >
    > >> CBFalconer <> 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
     
    Richard Bos, Sep 14, 2007
    #16
  17. bcpkh

    CBFalconer Guest

    jaysome wrote:
    > CBFalconer <> 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
     
    CBFalconer, Sep 14, 2007
    #17
  18. Keith Thompson <> writes:

    > Bart van Ingen Schenau <> 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.
     
    Ben Bacarisse, Sep 14, 2007
    #18
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Aaron

    duration

    Aaron, May 17, 2004, in forum: ASP .Net
    Replies:
    1
    Views:
    539
    Zuzar Lakdawala
    May 17, 2004
  2. Thomas

    User control cache duration

    Thomas, Jul 22, 2004, in forum: ASP .Net
    Replies:
    3
    Views:
    676
    Thomas
    Jul 23, 2004
  3. z. f.
    Replies:
    0
    Views:
    424
    z. f.
    Feb 10, 2005
  4. z. f.
    Replies:
    5
    Views:
    4,634
    Juan T. Llibre
    Feb 10, 2005
  5. Shawn H. Mesiatowsky

    ADO leaving Db connection open for duration of session

    Shawn H. Mesiatowsky, Mar 31, 2005, in forum: ASP .Net
    Replies:
    1
    Views:
    451
    Brock Allen
    Mar 31, 2005
Loading...

Share This Page