search

joan2937 / pigpio

pigpio is a C library for the Raspberry which allows control of the General Purpose Input Outputs (GPIO).
The Unlicense
1.45k stars 407 forks source link

Code Review #548

Open Frank-Friemel opened 1 year ago

Frank-Friemel commented 1 year ago

https://github.com/joan2937/pigpio/blob/c33738a320a3e28824af7807edafda440952c05d/pigpiod_if2.c#L345

Why is variable „got“ declared static?

1 . Seems to be needless (to be static)

  1. By declaring it static it‘s being shared by multiple callback-threads but I don‘t see a mutex to protect „got“
guymcswain commented 1 year ago

Is there a specific issue you've encountered that lead to your code review? I'd rather focus there first.

Frank-Friemel commented 1 year ago

I've ported the client code to Windows and stumbled over this line. I guess that multiple remote servers won't be connected by one client (usually), but if some user tries to do so ... I expect that it won't work

guymcswain commented 1 year ago

I guess that multiple remote servers won't be connected by one client (usually), but if some user tries to do so ... I expect that it won't work

It should. If I get some time over the next month or so I may try a simple test case. Would be great if you could generate a test case.

guymcswain commented 1 year ago

Actually, there is at least one outstanding issue with multiple clients that is not related to the specific static variable you've identified. The 'work around' for that issue, using the Python client, is to implement your threads behind the client. So single client but supporting multi threads so to speak.

Frank-Friemel commented 1 year ago

Well, you didn't answer my original question. Why is it declared "static"?

guymcswain commented 1 year ago

I don't know.