Menu

#13 -D and -U options broken in ctorrent 3.3.2

open
nobody
Client (14)
5
2009-03-14
2009-03-14
KuleRucket
No

I found this but when building ctorrent for a MIPS platform. When processing the -U and -D options, a strtod is used to read the options and the double is typecast to an (int). This fails leaving the options unset when viewed using <esc> 8 <enter>.

I checked the dtorrent svn and the bug still exists. The attached patches fix ctorrent 3.3.2.

Discussion

  • KuleRucket

    KuleRucket - 2009-03-14

    Patches to fix strtod typecast to int

     
  • dholmes999

    dholmes999 - 2009-03-15

    If casting a double value to an int doesn't work, that is a compiler bug. The changes you've made look fine, but if it is a general problem casting a double to any integer type then this will also impact other parts of the program (rate calculations come to mind). This works in other environments I've tested. Note that if CTCS is used to manage bandwidth then it will override the specified limits.

    What compiler and version were you using, and what values did you specify for the -D and -U options?

     
  • KuleRucket

    KuleRucket - 2009-03-16

    The environment is:
    gcc-4.0.4
    uClibc-0.9.28

    It is an old version of uClibc (forced by the platform I am working on), however I don't see any reason to cast strtod to and integer when strtol is available. That's what it's for after all. May using a <dynamic_cast> would be safer in any case.

    The problem is evident for any values of -U and -D. For example -D 20 -U 10.

     
  • dholmes999

    dholmes999 - 2009-03-17

    I think you may have missed my larger point. While you are correct that it isn't necessary to generate a double value here (the strtod call is from original ctorrent), it is not incorrect to cast the result to an int. There are other places in the program where casting a variable value or result of a calculation which is of type double to an integer type is used, and this will be a problem if your environment can't do that. For example Rate::RateAdd() uses "(time_t)m_history_last->timestamp" where the timestamp is of type double.

    My best recommendation is to test casting between various types, using static values, variables, calculations, and function return values. Testing these (in a test program) and ctorrent (in particular the strtod version of -D/-U option parsing since that is readily reproducible in your case) with -O0, -O1, and -O2 to check for different results is also useful.

    Check which floating-point code type being used also; the gcc man page I have handy indicates that with -msoft-float on mips, library support is required. A search turned up an (old) archived IRC log which appears to be among the uClibc developers, indicating that soft FP doesn't work with uClibc and mips; perhaps there is more detail or more current info available somewhere.

    Note that strtod is also used in the CTCS module to parse a non-integer option value, and there is no way to guarantee that strtod won't be needed for another future command-line option. Compiler bugs are rare but not unheard of; we seem to have isolated one in this project previously (inconsistent arithmetic with g++ on NSLU2).

     

Log in to post a comment.