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

Construct joystick pathnames correctly #743

Merged
merged 1 commit into from Nov 30, 2014

Conversation

Projects
None yet
5 participants
@ellenpoe
Contributor

ellenpoe commented Nov 27, 2014

Fixes #742

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Nov 27, 2014

Member

Your change in updatePluggedList() would mess up the udev code further down that relies on the name as well. udev joystick input nodes have to contain the preceding "js" as well.

I suggest simply applying this minimal patch instead (which focuses on fixing the oversight during my refactor):

diff --git a/src/SFML/Window/Unix/JoystickImpl.cpp b/src/SFML/Window/Unix/JoystickImpl.cpp
index 867d076..9d9ee81 100644
--- a/src/SFML/Window/Unix/JoystickImpl.cpp
+++ b/src/SFML/Window/Unix/JoystickImpl.cpp
@@ -49,7 +49,7 @@ namespace

         for (unsigned int i = 0; i < sf::Joystick::Count; ++i)
         {
-            std::ostringstream name("js");
+            std::ostringstream name("js", std::ios_base::ate);
             name << i;
             std::string nameString = name.str();

@@ -260,7 +260,7 @@ bool JoystickImpl::open(unsigned int index)
 {
     if (plugged[index])
     {
-        std::ostringstream name("/dev/input/js");
+        std::ostringstream name("/dev/input/js", std::ios_base::ate);
         name << index;

         // Open the joystick's file descriptor (read-only and non-blocking)
Member

binary1248 commented Nov 27, 2014

Your change in updatePluggedList() would mess up the udev code further down that relies on the name as well. udev joystick input nodes have to contain the preceding "js" as well.

I suggest simply applying this minimal patch instead (which focuses on fixing the oversight during my refactor):

diff --git a/src/SFML/Window/Unix/JoystickImpl.cpp b/src/SFML/Window/Unix/JoystickImpl.cpp
index 867d076..9d9ee81 100644
--- a/src/SFML/Window/Unix/JoystickImpl.cpp
+++ b/src/SFML/Window/Unix/JoystickImpl.cpp
@@ -49,7 +49,7 @@ namespace

         for (unsigned int i = 0; i < sf::Joystick::Count; ++i)
         {
-            std::ostringstream name("js");
+            std::ostringstream name("js", std::ios_base::ate);
             name << i;
             std::string nameString = name.str();

@@ -260,7 +260,7 @@ bool JoystickImpl::open(unsigned int index)
 {
     if (plugged[index])
     {
-        std::ostringstream name("/dev/input/js");
+        std::ostringstream name("/dev/input/js", std::ios_base::ate);
         name << index;

         // Open the joystick's file descriptor (read-only and non-blocking)
@ellenpoe

This comment has been minimized.

Show comment
Hide comment
@ellenpoe

ellenpoe Nov 27, 2014

Contributor

Makes sense. I didn't know about std::ios_base::ate. That's neat.

Contributor

ellenpoe commented Nov 27, 2014

Makes sense. I didn't know about std::ios_base::ate. That's neat.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Nov 27, 2014

Member

I personally prefer

std::ostringstream name;
name << "js" << i;

... which makes it clearer what string is constructed, and avoids all this constructor stuff.

Member

LaurentGomila commented Nov 27, 2014

I personally prefer

std::ostringstream name;
name << "js" << i;

... which makes it clearer what string is constructed, and avoids all this constructor stuff.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 27, 2014

Member

I'd also go with @LaurentGomila's version.

@nopoe Do you want to adjust your PR?

Member

eXpl0it3r commented Nov 27, 2014

I'd also go with @LaurentGomila's version.

@nopoe Do you want to adjust your PR?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Nov 27, 2014

Member
diff --git a/src/SFML/Window/Unix/JoystickImpl.cpp b/src/SFML/Window/Unix/JoystickImpl.cpp
index 867d076..0b8d479 100644
--- a/src/SFML/Window/Unix/JoystickImpl.cpp
+++ b/src/SFML/Window/Unix/JoystickImpl.cpp
@@ -49,8 +49,8 @@ namespace

         for (unsigned int i = 0; i < sf::Joystick::Count; ++i)
         {
-            std::ostringstream name("js");
-            name << i;
+            std::ostringstream name;
+            name << "js" << i;
             std::string nameString = name.str();

             int file = ::open(("/dev/input/" + nameString).c_str(), O_RDONLY);
@@ -260,8 +260,8 @@ bool JoystickImpl::open(unsigned int index)
 {
     if (plugged[index])
     {
-        std::ostringstream name("/dev/input/js");
-        name << index;
+        std::ostringstream name;
+        name << "/dev/input/js" << index;

         // Open the joystick's file descriptor (read-only and non-blocking)
         m_file = ::open(name.str().c_str(), O_RDONLY | O_NONBLOCK);

😛

Member

binary1248 commented Nov 27, 2014

diff --git a/src/SFML/Window/Unix/JoystickImpl.cpp b/src/SFML/Window/Unix/JoystickImpl.cpp
index 867d076..0b8d479 100644
--- a/src/SFML/Window/Unix/JoystickImpl.cpp
+++ b/src/SFML/Window/Unix/JoystickImpl.cpp
@@ -49,8 +49,8 @@ namespace

         for (unsigned int i = 0; i < sf::Joystick::Count; ++i)
         {
-            std::ostringstream name("js");
-            name << i;
+            std::ostringstream name;
+            name << "js" << i;
             std::string nameString = name.str();

             int file = ::open(("/dev/input/" + nameString).c_str(), O_RDONLY);
@@ -260,8 +260,8 @@ bool JoystickImpl::open(unsigned int index)
 {
     if (plugged[index])
     {
-        std::ostringstream name("/dev/input/js");
-        name << index;
+        std::ostringstream name;
+        name << "/dev/input/js" << index;

         // Open the joystick's file descriptor (read-only and non-blocking)
         m_file = ::open(name.str().c_str(), O_RDONLY | O_NONBLOCK);

😛

@eXpl0it3r eXpl0it3r added this to the 2.2 milestone Nov 27, 2014

@eXpl0it3r eXpl0it3r self-assigned this Nov 27, 2014

@eXpl0it3r eXpl0it3r removed the s:unassigned label Nov 27, 2014

@ellenpoe

This comment has been minimized.

Show comment
Hide comment
@ellenpoe

ellenpoe Nov 27, 2014

Contributor

Ok, it should now match @LaurentGomila's proposed fix.

Contributor

ellenpoe commented Nov 27, 2014

Ok, it should now match @LaurentGomila's proposed fix.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 27, 2014

Member

Can you squash the commits?

Member

eXpl0it3r commented Nov 27, 2014

Can you squash the commits?

@ellenpoe

This comment has been minimized.

Show comment
Hide comment
@ellenpoe

ellenpoe Nov 28, 2014

Contributor

Ok hopefully that worked. I've never done that before.

Contributor

ellenpoe commented Nov 28, 2014

Ok hopefully that worked. I've never done that before.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 28, 2014

Member

Looks good to me. 👍

Member

Bromeon commented Nov 28, 2014

Looks good to me. 👍

@eXpl0it3r eXpl0it3r merged commit 7ee0734 into SFML:master Nov 30, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment