Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed payload functions from int to qlonglong in TorrentHandle class #3990

Merged
merged 1 commit into from
Oct 27, 2015

Conversation

naikel
Copy link
Contributor

@naikel naikel commented Oct 25, 2015

This fixes #3985.

Functions totalPayloadUpload() and totalPayloadDownload() must be qlonglong since total_payload_upload and total_payload_download are defined as boost::int64_t size in libtorrent.

@DoumanAsh
Copy link
Contributor

I think it is better to use qint64 as it is more clear and short comparing to qlonglong
And well they are the same :)

@naikel
Copy link
Contributor Author

naikel commented Oct 26, 2015

Maybe, but I tried to follow the coding style. You can see other functions like totalDownload() and totalUpload() are qlonglong.

@@ -1055,12 +1055,12 @@ int TorrentHandle::downloadPayloadRate() const
return m_nativeStatus.download_payload_rate;
}

int TorrentHandle::totalPayloadUpload() const
qlonglong TorrentHandle::totalPayloadUpload() const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not it be better to use libtorrent::size_type directly or via typedef instead of qlonglong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. The typedef size_type works up to 1.0.6, but is removed in libtorrent 1.0.7, and they reference boost::int64_t.

They removed it because a conflict with std::string::size_type. I guess it caused confusion or something.

Anyway boost::int64_t is a signed int64 and it's the same as a qint64 or a qlonglong.

Remember we would have to change functions that are already there like torrentUpload() and torrentDownload() that are already defined as qlonglong, they just missed the Payload ones...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. Then why not qint64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the hate of qlonglong :) I'm with you all I would have used qint64 but qlonglong was already there in totalDownload() and totalUpload() and that's not my code ;) So I tried to follow the code style that was already there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI: qlonglong is the same as qint64
qint64 is just better name of this type so it is up to you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you use qlonglong here?

Do you mean "Why didn't you use"?

could you comment on this, please?

Any comments? It's just my mistake. At the time I had processed a huge amount of code...
Why didn't I find this earlier? I use x64 environment where int and qlonglong are the same size.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nothing wrong with qlonglong as it is the same as qint64.
At least according to QT docs.

There is not need to change it all at once, but slowly with new commits

And please no need to blame anyone about it.
After all usage of qlonglong is not a bad thing as it is equal to qint64

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DoumanAsh Why change qlonglong to qint64?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no technical need, it is just more easy to understand that this is 64bit integer.
So to improve code read-ability.
But you might as well not to bother about it.

@naikel Please do not worry about what types are used in other places.
qlonglong and qint64 are the same.
Do what you think right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's right as it is. Maybe a different commit could improve readibility changing all qlonglong to qint64.

@sledgehammer999
Copy link
Member

@naikel Thanks for fix. @ all qlonglong is ok for now.

sledgehammer999 added a commit that referenced this pull request Oct 27, 2015
Changed payload functions from int to qlonglong in TorrentHandle class
@sledgehammer999 sledgehammer999 merged commit 99ba8a6 into qbittorrent:master Oct 27, 2015
@naikel naikel deleted the payload_fix branch January 3, 2016 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session download shows "Unknown this session" when session downloaded > sizeof(int)
5 participants