#356 playerc_client_read return fix

Release-2.1
closed-accepted
Player (393)
5
2009-01-06
2008-10-03
Kevin Barry
No

The documentation states that playerc_client_read will return NULL on error, however sometimes it returns -1. This is due to someone trying to add a check so it will return NULL when the socket is invalid (client->sock < 0) however it was added to the wrong if statement. This patch fixes it.

This also fixes libplayerc++ as the error checking is done against libplayerc. So programs like playerjoy give "playerc warning : warning : no socket to peek at" in a tight loop.

release-2-1-patches

Discussion

  • Kevin Barry

    Kevin Barry - 2008-10-03

    Actually now that I look at this, why is this function implemented the way it is? I think this could be better implemented using the client_peek call, which already does a blocking poll, rather than a test/sleep loop. This also would make it trivial to implement a playerc_client_read_timeout. I'll see about changing this and post back. Please let me know if there is a reason playerc_client_read is implemented the way it is.

     
  • Nobody/Anonymous

    Okay I've looked into the playerc_client_read. I think it may have been a left over from a previous architecture.

    A peek does not guarantee enough data for playerc_client_read_withproxy to return 1, so a playerc_client_read_timeout is not as easy as I thought.

    However, I do think I have a cleaner method of implementing playerc_client_read, that removes the latency from the 10ms sleep.

     
  • G Biggs

    G Biggs - 2008-10-07

    Can you clarify which of these patches should be applied?

     
  • Kevin Barry

    Kevin Barry - 2008-10-07

    playerc_client_read_peek.diff should be applied.

     
  • Toby Collett

    Toby Collett - 2009-01-04
    • milestone: --> Release-2.1
     
  • Toby Collett

    Toby Collett - 2009-01-06
    • status: open --> closed-accepted
     
  • Toby Collett

    Toby Collett - 2009-01-06

    The patch was a little messy since it still had the read_timeout code in it. I have applied what I believe is equivalent functionality to both 2-1 and trunk, can you try it out and confirm it works for you.

    On a less related note, the read_timeout you were playing with should work as in pull mode the client structure stores enough state to recover an interrupted read (at least it should, would need some testing). Feel free to submit a patch against trunk if you would like to see that method included.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks