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

CConnection: Modify UserPasswdGetter and UserMsgBox interface to CConnection #1794

Closed
wants to merge 1 commit into from

Conversation

KangLin
Copy link
Contributor

@KangLin KangLin commented Jul 30, 2024

CConnection: Modify UserPasswdGetter and UserMsgBox interface to CConnection

Problems with the original code: A process can only establish one connection.
After modification, multiple connections can be supported.

@CendioOssman
Copy link
Member

Multiple connections is not something vncviewer can do presently, so I'm unclear for the need to refactor this?

@KangLin
Copy link
Contributor Author

KangLin commented Jul 30, 2024

But I need multiple connections. So refactored it.
It's good for degree architecture.

@CendioOssman
Copy link
Member

Ah, that's right. You were using TigerVNC as a library.

I just want to reiterate that this is not something we support, so it's best if you can align your needs with those of the TigerVNC project.

That said, I see no need for these callbacks to be global, so this clean-up should be acceptable.

common/rfb/CConnection.h Outdated Show resolved Hide resolved
tests/perf/decperf.cxx Show resolved Hide resolved
tests/perf/encperf.cxx Outdated Show resolved Hide resolved
@KangLin
Copy link
Contributor Author

KangLin commented Aug 6, 2024

I'm glad you can accept tigervnc as a library that supports multiple operating systems. I will help you achieve it.

@KangLin KangLin force-pushed the password branch 3 times, most recently from b81551f to 406dbb2 Compare August 7, 2024 04:34
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

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

Almost there. Just a couple small tweaks and I can merge this.

common/rfb/CConnection.h Outdated Show resolved Hide resolved
common/rfb/CConnection.h Outdated Show resolved Hide resolved
@KangLin KangLin force-pushed the password branch 4 times, most recently from 7607ce2 to 420a2b1 Compare August 12, 2024 07:40
common/rfb/CConnection.h Outdated Show resolved Hide resolved
common/rfb/CSecurityRSAAES.cxx Outdated Show resolved Hide resolved
common/rfb/CSecurityTLS.cxx Outdated Show resolved Hide resolved
vncviewer/UserDialog.h Show resolved Hide resolved
common/rfb/CConnection.h Outdated Show resolved Hide resolved
@KangLin KangLin force-pushed the password branch 3 times, most recently from c1a5df5 to 28d9039 Compare August 14, 2024 02:08
common/rfb/CSecurityTLS.cxx Outdated Show resolved Hide resolved
common/rfb/CConnection.h Outdated Show resolved Hide resolved
vncviewer/UserDialog.h Show resolved Hide resolved
common/rfb/CConnection.h Outdated Show resolved Hide resolved
@KangLin KangLin force-pushed the password branch 3 times, most recently from 70316e6 to 4b8f263 Compare August 16, 2024 02:03
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

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

This is looking really good now. Nice work!

Just two small style changes and I can go ahead and merge this.

vncviewer/CConn.h Outdated Show resolved Hide resolved
common/rfb/CConnection.h Outdated Show resolved Hide resolved
@KangLin KangLin force-pushed the password branch 3 times, most recently from b43da5b to 89c05d8 Compare August 20, 2024 00:45
…nection

Problems with the original code: A process can only establish one connection.
After modification, multiple connections can be supported.
@CendioOssman
Copy link
Member

Committed as 1b0387a. Thanks!

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.

2 participants