PHP-Serial icon indicating copy to clipboard operation
PHP-Serial copied to clipboard

Logic Error in readPort

Open tvleavitt opened this issue 11 years ago • 4 comments

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.

tvleavitt avatar Feb 12 '15 18:02 tvleavitt

I have changed the else body to

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

It works pretty well ;-)

stollr avatar May 12 '21 07:05 stollr

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

tvleavitt avatar May 12 '21 20:05 tvleavitt

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.

stollr avatar May 13 '21 18:05 stollr

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.

tvleavitt avatar May 13 '21 19:05 tvleavitt