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

Logic Error in readPort #13

Open
tvleavitt opened this issue Feb 12, 2015 · 4 comments
Open

Logic Error in readPort #13

tvleavitt opened this issue Feb 12, 2015 · 4 comments

Comments

@tvleavitt
Copy link

readPort will always return more characters than the amount requested. If you iterate through this loop, you'll see this. $i is the # of characters read off the serial port, which is equal to the strlen of $content until the loop exits. Let's take a request for 256 characters as an example.

1st loop: $i = 0, $count = 256; if/else loop in do-while falls through to else
2nd loop: $i = 128, $count = 256; if/else loop falls through to else
3rd loop: $i = 256 (requested amount; $count = 256; if/else loop falls through to else, because if statement is "if $i is greater than $count, then execute", and the condition for exiting the do/while loop is $i must not equal the length of $content.
4th loop: $i = 384, $count = 256; if loop triggered; function attempts to set the length of $content to $count by passing a negative value to fread, which, as far as I can tell, fails to actually do this. The value of $content is now some random value that is greater than $count, up to 384 characters, and definitely over the value of count. I've reproduced this on Ubuntu 12.04 and 14.04.

This causes the do / while condition to fail, and the loop exits (unless the value of $content is exactly 384, in which case, the loop will iterate again, and add another 128 characters).

         $content = ""; $i = 0;

         if ($count !== 0) {
             do {
                 if ($i > $count) {
                     $content .= fread($this->_dHandle, ($count - $i));
                 } else {
                     $content .= fread($this->_dHandle, 128);
                 }
             } while (($i += 128) === strlen($content));
         } else {
             do {
                 $content .= fread($this->_dHandle, 128);
             } while (($i += 128) === strlen($content));
         }

         return $content;

The fix is to restructure this to use some other logic structure, but, in the interim, if you change the logic of the if loop to "if ($i >= $count)", the loop will exit and return the requested number of characters. Note: this fix will break a ton of code that unknowingly depends on this function returning more than the requested number of characters, which, especially in serial communications, when you're dealing with small chunks of data, could result in a lot of situations where less than 128 bytes of extra characters are being pulled.

@stollr
Copy link

stollr commented May 12, 2021

I have changed the else body to

$content = stream_get_contents($this->_dHandle);

It works pretty well ;-)

@tvleavitt
Copy link
Author

It's six years later... is anyone maintaining this code at all?

@stollr
Copy link

stollr commented May 13, 2021

No ^^ but may be there are people still using it. I have started using it some days ago for a project. And it's quite helpful.

@tvleavitt
Copy link
Author

If you feel comfortable that you've implemented the right solution, I suggest that you create a pull request, and maybe "fork" the code so that a version with this update is available for download by folks.

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

No branches or pull requests

2 participants