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

GDB server: support for local socket file #4686

Closed
wants to merge 15 commits into from
Closed

GDB server: support for local socket file #4686

wants to merge 15 commits into from

Conversation

Daniel-Valentine
Copy link
Contributor

@Daniel-Valentine Daniel-Valentine commented Jun 1, 2018

NOTE: not ready for merge; requires:

  • further testing
    -- especially testing local sockets in Windows, currently disabled
    -- in addition, this modifies the config yaml (read below)
  • logic to unlink socket files
  • rebase
  • code cleanup

Right now, the GDB server only listens on an IPv4 port, but it might make sense to include the option to listen at a local socket. May be preferable to a IPv4 since a user won't have to specify special firewall rules.

Note that this also requires how config.yml encodes the GDB server settings; for now, they're in a separate heading, e.g.

GDB debug server:
  Enabled: true
  Socket type: UNIX 
  IPv4 port: 2345
  UNIX socket file path: /tmp/rpcs3.gdb.socket

whereas RPCS3 just has a single key/value for IPv4 port in the yaml under misc:

Miscellaneous:
  Automatically start games after boot: true
  Exit RPCS3 when process finishes: false
  # ...snip...
  Port: 2345

@@ -3,6 +3,7 @@
#include "VFS.h"
#include "Utilities/Atomic.h"
#include "Utilities/Config.h"
#include <sys/socket.h>
Copy link
Member

Choose a reason for hiding this comment

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

Get it somewhere out of there

cfg::string socket_type{this, "Socket type", "INET"};
cfg::_int<1, 65535> ipv4_port{this, "IPv4 port", 2345};
cfg::string unix_fpath{this, "UNIX socket file path",
#ifndef _WIN32
Copy link
Member

Choose a reason for hiding this comment

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

Makes more sense to hide the whole config string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarify what you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

he means remove the option completely on windows

@Megamouse
Copy link
Contributor

Needs a rebase with fixing conflicts and handling code reviews

@Nekotekina
Copy link
Member

Closing as it's now implemented in a very different manner.

@Nekotekina Nekotekina closed this Oct 9, 2019
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.

4 participants