I don’t believe that is the case, if I use the code outputs[POLY_CHANNELS_OUTPUT].setChannels(15); then there are only 15 channels output, I have tested this using the VCV Split module.
the code outputs[POLY_CHANNELS_OUTPUT].setChannels(16); does work, it does what I want, it makes the output send 16 channels, but it also generates the warning (its not an error).
Its not really a big deal, but it could indicate there are other undiscovered bugs lurking in the API related to the number of channels.
While you are correct that arrays are zero indexed,
array sizes do not align with their index
void setChannels(int channels) {
// If disconnected, keep the number of channels at 0.
if (this->channels == 0) {
return;
}
// Set higher channel voltages to 0
for (int c = channels; c < this->channels; c++) {
voltages[c] = 0.f;
}
// Don't allow caller to set port as disconnected
if (channels == 0) {
channels = 1;
}
this->channels = channels;
}
this->channels = PORT_MAX_CHANNELS = 16
so the loop for (int c = channels; c < this->channels; c++) { … } will never be executed when channels = 16
Looking at Port.hpp it’s pretty clear there is a flaw, of sorts. If you ever called setChannels with a big number it would happily accept it. Then, if you later called it with a different number it would index into that array with (big number - 1) and presumably crash.
Of course if you never call it with a silly value that won’t happen. Still, perhaps setChannels should rejects out of range numbers. Presumably a negative number would be a catastrophe.
No, the code is correct. The goal of this loop is to set the voltages of all unused channels to zero. If all channels are used (e.g. in case of your module), there’s nothing to do, and therefore the body of the loop will never be executed.
If your module uses 15 channels, then the body of the loop will be executed once for the 16th channel.
Btw. you can suppress specific compiler warnings or use workarounds (see Module.hpp, lines 228-230).
No, I don’t think so. Look at that loop and imagine what would happen if this->channels == 2000, or even worse this->channels == -1. I think the code works fine, as long as a plugin never sets the channels to something crazy.
Yes, the code is correct, but it would be robust, too, if setChannels rejected out of range numbers. That’s that what the warning is saying - the compiler doesn’t know that the caller never sets the channels to a bad value.
My preferred method of handling this would be to use an assertion, I don’t see the point of having code like that in anything other than a debug build, but then a lot of my career involved counting clock cycles in real time code when things ran at MHz as opposed to GHz.