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.

Sign up for GitHub

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

Jump to bottom

fixup: dedup tr_rpc_address with tr_address #5523

Merged
merged 2 commits into from
Jun 19, 2023
Merged

fixup: dedup tr_rpc_address with tr_address #5523

merged 2 commits into from
Jun 19, 2023

Conversation

LaserEyess
Copy link
Contributor

Still needs to be tested. I'd also like to make some actual tests for this, but I'm a little unsure how the testing framework works.

@LaserEyess LaserEyess force-pushed the fixup_rpc branch 2 times, most recently from d4fceb7 to fc83319 Compare May 13, 2023 19:25
ckerr
ckerr reviewed May 13, 2023
View reviewed changes
libtransmission/rpc-server.cc Outdated Show resolved Hide resolved
libtransmission/rpc-server.cc Outdated Show resolved Hide resolved
libtransmission/rpc-server.cc Outdated Show resolved Hide resolved
libtransmission/rpc-server.cc Outdated Show resolved Hide resolved
libtransmission/rpc-server.cc Outdated Show resolved Hide resolved
@LaserEyess
Copy link
Contributor Author

LaserEyess commented May 13, 2023 •
edited
Loading

I moved tr_rpc_address to be a class with private fields for the address as well, with helper methods like is_unix_addr() and is_inet_addr(). I think is_valid() and set_inaddr_any() are redundant now. tr_rpc_address is now always initialized as INADDR_ANY, which is the default anyway.

Let me know if this approach is acceptable. FYI, my motivation behind this is that I'm eventually interested in doing #4315

@ckerr ckerr self-requested a review May 14, 2023 05:57
@LaserEyess LaserEyess marked this pull request as ready for review May 29, 2023 18:35
@LaserEyess @ckerr
fixup: dedup tr_rpc_address with tr_address
bc05872 
tr_rpc_address was duplicating a lot of work that tr_address was doing a
lot better. Fall back to using tr_address for ipv4/ipv6 connections and
make a tr_unix_addr for handling unix sockets.
@LaserEyess @ckerr
remove unnecessary functions
28a3ab3
ckerr
ckerr approved these changes Jun 19, 2023
View reviewed changes
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

@LaserEyess LGTM, sorry for the delay in the followup review. FWIW it's not you or anything to do with this PR; I've just been super busy with non-Transmission stuff. Please do keep contributing 💚

@ckerr ckerr added type:refactor A code change that neither fixes a bug nor adds a feature semver:minor adds functionality in a backwards compatible manner labels Jun 19, 2023
@ckerr ckerr added this to the 4.1.0 milestone Jun 19, 2023
@ckerr ckerr merged commit 23a52fa into transmission:main Jun 19, 2023
21 checks passed
ckerr added a commit that referenced this pull request Jun 19, 2023
@ckerr
Squashed commit of the following:
3519acf 
commit 4b9b992
Author: Charles Kerr <charles@charleskerr.com>
Date:   Mon Jun 19 16:51:34 2023 -0500

    refactor: prefer std::map over QMap in transmission-qt (#5641)

    * refactor: use std::map instead of QMap in PrefsDialog.cc

    * refactor: use std::map instead of QMap in DetailsDialog.cc

    * refactor: use std::map instead of QMap in OptionsDialog.cc

    * refactor: use std::map instead of QMap in FileTreeModel.cc

commit 040bc8a
Author: Dmitry Antipov <dmantipov@yandex.ru>
Date:   Tue Jun 20 00:22:54 2023 +0300

    fix: Qt 6.5 deprecation warning (#5552)

commit 23a52fa
Author: LaserEyess <16581533+LaserEyess@users.noreply.github.com>
Date:   Mon Jun 19 16:51:20 2023 -0400

    fixup: dedup tr_rpc_address with tr_address (#5523)

    * fixup: dedup tr_rpc_address with tr_address

    tr_rpc_address was duplicating a lot of work that tr_address was doing a
    lot better. Fall back to using tr_address for ipv4/ipv6 connections and
    make a tr_unix_addr for handling unix sockets.

    * remove unnecessary functions

commit ddac059
Author: Василий Чай <basilefff@gmail.com>
Date:   Mon Jun 19 09:30:55 2023 +0400

    fix: return error when renaming into existing file (#5563)

commit b8ff35c
Author: tearfur <46261767+tearfur@users.noreply.github.com>
Date:   Mon Jun 19 08:06:31 2023 +0800

    handle IPv6 NAT during LTEP handshake (#5565)

    * fix: peer handshake reported wrong ipv6 address

    #5542 (comment)

commit fd583ac
Author: Charles Kerr <charles@charleskerr.com>
Date:   Sun Jun 18 17:36:39 2023 -0500

    deps: bump libfmt to v10.0.0 (#5635)

    seems to be semver/minor safe for our API use

    Fixes #5511.

    Possibly fixes #5627.

commit 1664088
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Sun Jun 18 15:55:43 2023 -0500

    chore: update generated transmission-web files (#5588)

commit 0fd7989
Author: Gary Elshaw <69029666+GaryElshaw@users.noreply.github.com>
Date:   Mon Jun 19 07:11:56 2023 +1200

    fix: restore png files that were corrupted in recent "compression" script

commit bd9d110
Author: Cœur <coeur@gmx.fr>
Date:   Sun Jun 18 20:32:34 2023 +0200

    fix 'setNeedsDisplay' is deprecated: first deprecated in macOS 10.14 (#5633)

commit 76166d8
Author: Cœur <coeur@gmx.fr>
Date:   Mon Jun 12 18:03:22 2023 +0200

    refactor: replace NSMutableDictionary with constant attributes (#5221)

commit c379cd7
Author: Robin Seth Ekman <robin.seth.ekman@gmail.com>
Date:   Mon Jun 12 03:19:54 2023 +0200

    fix: transmission-remote only list every other label (fixes #5571) (#5572)

commit 87f254a
Author: Charles Kerr <charles@charleskerr.com>
Date:   Sun Jun 11 18:52:45 2023 -0500

    chore: bump fast_float snaapshot to 5.2.0 (#5605)

commit 802619e
Author: tearfur <46261767+tearfur@users.noreply.github.com>
Date:   Mon Jun 12 07:27:01 2023 +0800

    fix: fixes and improvements to global IP query (#5510)

commit c8e84f8
Author: Charles Kerr <charles@charleskerr.com>
Date:   Sun Jun 11 16:28:43 2023 -0500

    ci: remove "brew update" step (#5606)

commit 60c68af
Author: Charles Kerr <charles@charleskerr.com>
Date:   Sun Jun 11 11:25:35 2023 -0500

    Revert "fix: some labels not displayed in transmission-remote (#5600)"

    This reverts commit ebd5080.

commit ebd5080
Author: Charles Kerr <charles@charleskerr.com>
Date:   Sat Jun 10 19:01:32 2023 -0500

    fix: some labels not displayed in transmission-remote (#5600)

    Fixes #5571.

commit 8ca02b8
Author: Mike Gelfand <mikedld@users.noreply.github.com>
Date:   Sun Jun 11 02:09:18 2023 +0400

    Avoid locale use in `tr_truncd()` (#5587)

    `tr_parseNum<>()` is implemented with `fast_float::from_chars()`, and
    the latter is documented as "expecting a locale-indepent format
    equivalent to what is used by std::strtod in the default ("C") locale".
    To accomodate locale independent number parsing, switch back to
    locale-independent number formatting in `tr_truncd()` by both removing a
    `L` format specifier from `fmt::format_to_n()` call and using a fixed
    "." decimal separator when truncating the value.

commit 0ef58c2
Author: Charles Kerr <charles@charleskerr.com>
Date:   Mon Jun 5 17:03:11 2023 -0500

    chore: improve lossless compression of png files (#5586)

commit c1c27f3
Author: Daniel Kamil Kozar <dkk089@gmail.com>
Date:   Mon Jun 5 20:15:32 2023 +0200

    Expose files' begin and end pieces via RPC (#5578)

    * Expose files' begin and end pieces via RPC

    This adds two arguments, `beginPiece` and `endPiece`, for each of the entries
    in the `files` array of the RPC's `torrent-`get` method.

    The point is to allow RPC clients to display a file's completion progress as
    piece-based in addition to byte-based.

commit 61679e1
Author: Cœur <coeur@gmx.fr>
Date:   Mon May 29 15:43:43 2023 +0200

    Support localized punctuation for "Port:" (#4452)

    * Support localized punctuation for "Port:"

    * Code review: adopting "Port: %@"

    * updating other locales for "Port" -> "Port: %@"
LaserEyess added a commit to LaserEyess/transmission that referenced this pull request Jun 24, 2023
@LaserEyess
fix: fix broken unix socket support
aec5742 
The changes in transmission#5523 made two errors in the new implementation

1. tr_unix_address::to_string() got the ternary check backwards, leading
   to always printing an empty string when the address is valid.
2. The inputs to tr_strvStartsWith in tr_unix_address::from_string()
   were backwards as well, leading to the check failing for valid
   socket addresses.
@LaserEyess LaserEyess mentioned this pull request Jun 24, 2023
Merged
ckerr pushed a commit that referenced this pull request Jun 24, 2023
@LaserEyess
fix: broken unix socket support (#5665)
9b5a755 
The changes in #5523 made two errors in the new implementation

1. tr_unix_address::to_string() got the ternary check backwards, leading
   to always printing an empty string when the address is valid.
2. The inputs to tr_strvStartsWith in tr_unix_address::from_string()
   were backwards as well, leading to the check failing for valid
   socket addresses.

Co-authored-by: LaserEyess <LaserEyess@users.noreply.github.com>
@ckerr ckerr added the notes:none Should not be listed in release notes label Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ckerr ckerr ckerr approved these changes

Assignees
No one assigned
Labels
notes:none Should not be listed in release notes scope:core semver:minor adds functionality in a backwards compatible manner type:refactor A code change that neither fixes a bug nor adds a feature
Milestone
4.1.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@LaserEyess @ckerr @Coeur