PlusToolkit / ndicapi

A common C API for communicating with NDI Polaris and Aurora devices
MIT License
47 stars 31 forks source link

Issue #24 we don't need a 2 dimonsional array for devicenames #25

Closed thompson318 closed 3 years ago

thompson318 commented 3 years ago

I couldn't see that having an array of device names was necessary, as it only gets written to if i==j, so I've just changed the devicename to a 1 dimensional array and continue the while loop until j<i+1. It seems to work for me see here

adamrankin commented 3 years ago

Unless it's necessary I wouldn't change code that isn't clear. It may be that it behaves differently on a different OS or something. I'd just leave it

lassoan commented 3 years ago

This is old, painful, plain C. You don't return std::string but a pointer to a C array. The current implementation takes care of memory management of the strings by preallocating all possible variations: the returned string remains valid forever, the ownership of the buffer is not transferred to the caller. If you overwrite the same memory buffer with different string at each connection then it can cause problems (it changes content of previously returned strings).

I would recommend to implement either of these options:

thompson318 commented 3 years ago

OK, that makes sense, thanks for looking into it. I've now fixed my issue downstream by using pyserial for discovering the serial ports instead, which is probably more elegant, so I don't think it's worth spending much effort trying to modernise this particular bit of code.