Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2379 closed Bug (fixed)

Infinite ratio shows as "nan", invalid JSON

Reported by: Maxim Owned by: charles
Priority: Normal Milestone: 1.76
Component: libtransmission Version: 1.74
Severity: Major Keywords:
Cc: maxim@…

Description

I have a few torrents with an upload ratio of ∞. This is being serialised (in the transmission-remote --list response, for example) as

{
  "error":0,
  "errorString":"",
  "eta":-1,
  "id":1,
  "leftUntilDone":0,
  "name":"redacted",
  "peersGettingFromUs":0,
  "peersSendingToUs":0,
  "rateDownload":0,
  "rateUpload":0,
  "sizeWhenDone":8072340340,
  "status":16,
  "uploadRatio":nan
}

"nan" is not valid JSON - the standard has no way to represent nan, inf, -inf, etc. It is however a common JSON extension to use them, sadly it's not supported by JSON_parser.c or by jQuery.

The serialisation of doubles seems to be falling through to vsnprintf() via evutil_vsnprintf(). A C99-compliant compiler will format NaN via %f to a string starting with "nan". Some older ones seem to serialise to a wacky number instead.

The result is that you can't list torrents in transmission-remote, the web UI fails, third-party tools fail (tr.net), and that's all the tools I have to test.

Attachments (2)

info.txt (8.2 KB) - added by Maxim 12 years ago.
Output of transmission-remote -t1 --info --debug
info.2.txt (6.6 KB) - added by Maxim 12 years ago.
[transmission-remote --torrent x --info --debug] - shows "Ratio: nan"

Download all attachments as: .zip

Change History (18)

comment:1 Changed 12 years ago by charles

We shouldn't ever be getting to the NaN stage anyway. Transmission has symbolic ratio values -1 and -2 for N/A and Infinity.

  • Are you certain that this is 1.74 you're using? The server typically doesn't include whitespace and linefeeds in the json it sends out via http anymore.
  • Could you please include the output from transmission-remote -t1 --info --debug, minus the hashString and name?

Changed 12 years ago by Maxim

Output of transmission-remote -t1 --info --debug

comment:2 Changed 12 years ago by Maxim

Should be 1.74. I pretty-printed the JSON for readability. I attached mostly-anonymised output as requested. --info works fine, it's --list that gets the ratio serialised - I think --info calculates it itself.

Changed 12 years ago by Maxim

[transmission-remote --torrent x --info --debug] - shows "Ratio: nan"

comment:3 follow-up: Changed 12 years ago by charles

Hm, not sure what's going on here.

If I hardwire transmission-remote to think that one of my torrents' downloadedEver is 2921338817 and uploadedEver is 1040315350, I get

  Downloaded: 2.7 GB
  Uploaded: 992.1 MB
  Ratio: 0.35
  • I wonder if it could have anything to do with tr_truncd() in libtransmission/utils.c. What happens if you stub that function out to simply return its x argument?
  • Could you add an "fprintf (stderr, "ratio is %f", ratio);" in strlratio right before the call to "return strlratio2" and see if that number's a nan too?

comment:4 Changed 12 years ago by Maxim

I'll have a go at compiling, it'll be a bit of an adventure as I've never compiled on my NAS before. Is there a guide?

comment:5 Changed 12 years ago by Maxim

Well, I found the guide, but it looks like I'll need to set up a Linux VM to cross-compile to ts72xx, which will require some time investment. For what it's worth, it was fine in 1.73.

comment:6 in reply to: ↑ 3 ; follow-up: Changed 12 years ago by Maxim

After many hours of struggling, I managed to cross-compile...

  • I wonder if it could have anything to do with tr_truncd() in libtransmission/utils.c. What happens if you stub that function out to simply return its x argument?

The JSON nans go away. The call to pow() is the problem. It is actually returning the ratio passed in on the previous call to tr_truncd(), rather than anything to do with the given parameters. Since the ratio is often less than 1, this gets truncated to 0 and we then divide by it soon after.

  • Could you add an "fprintf (stderr, "ratio is %f", ratio);" in strlratio right before the call to "return strlratio2" and see if that number's a nan too?

No nans in there, -2s as expected.

In --list almost all the ratios are showing as 2147483648. This has been the case for a lot of previous versions (I forget exactly), it never bothered me too much.

If you think it's because of the cross-compiling and Optware-ness, it might be better to pass it on to the maintainer on that side.

comment:7 in reply to: ↑ 6 Changed 12 years ago by charles

Replying to Maxim:

  • I wonder if it could have anything to do with tr_truncd() in libtransmission/utils.c. What happens if you stub that function out to simply return its x argument?

The JSON nans go away. The call to pow() is the problem. It is actually returning the ratio passed in on the previous call to tr_truncd(), rather than anything to do with the given parameters. Since the ratio is often less than 1, this gets truncated to 0 and we then divide by it soon after.

pow() is ignoring the arguments passed into it? pow(10,2) doesn't return 100? Am I reading this comment correctly?

comment:8 Changed 12 years ago by charles

What happens if we just avoid calling pow() altogether, by rewriting tr_truncd:

double
tr_truncd( double x, int decimal_places )
{
    static const int multiplier[] = { 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000 };
    const int64_t i = multiplier[decimal_places];
    double x2 = (int64_t)(x*i);
    return x2 / i;
}

comment:9 follow-up: Changed 12 years ago by charles

Maxim: ping?

comment:10 in reply to: ↑ 9 Changed 12 years ago by Maxim

  • Cc maxim@… added

Replying to charles:

Maxim: ping?

Pong!

Yes, it truly gave nonsense results. I put a block of printf(pow(const, const)) and it gave the same result for each. I guess it's a compiler weirdness, but odd that it happens on different machines.

With the pow()-less version it works indistinguishably from the stubbed one that doesn't truncate. It may need some protection from decimal_places being outside the multiplier[] array bounds, of course.

I'm not sure I understand the use of the function - surely decimal places are chosen for display or serialisation, why truncate internally, especially given that the last DP might end up double-truncated due to floating point rounding?

transmission-remote --list still shows bad ratios but I think that is a separate issue.

comment:11 Changed 12 years ago by charles

I don't know what to think about this ticket anymore. :|

It seems like we should just replace the pow() call since a replacement is easy, slightly faster, and we only have a limited range of decimal places that we'll ever want anyway.

But I don't understand how optware could possibly ship with a broken pow(). pow() has been in ANSI C forever.

comment:12 Changed 12 years ago by livings124

If a function as basic as pow() is open, I don't think it should be our place to ship our own custom implementation. I bet the built-in one has optimizations on a lot of systems.

comment:13 Changed 12 years ago by livings124

broken, no open ;)

comment:14 Changed 12 years ago by charles

  • Keywords backport added
  • Milestone changed from None Set to 1.80
  • Status changed from new to assigned

tr_truncd fixed in trunk by r9177 for 1.80

comment:15 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:16 Changed 12 years ago by charles

  • Keywords backport removed
  • Milestone changed from 1.80 to 1.76

Backported to the 1.7x branch for 1.76 in r9360

Note: See TracTickets for help on using tickets.