Skip to content

Conversation

@zfields
Copy link
Contributor

@zfields zfields commented Aug 17, 2017

The current implementation pulls bytes out of the sysex buffer, regardless of whether or not the user specified such bytes.

@fpistm
Copy link
Contributor

fpistm commented Aug 17, 2017

Access to argv[1] before test argc is risked? no ?

if (argc > 1) {
      delayTime = (argv[0] + (argv[1] << 7));
      if (delayTime > 0) {
        i2cReadDelayTime = delayTime;
      }
}

@zafields
Copy link

In this circumstance, there is no risk, because argv is a statically allocated buffer:

uint8_t parserBuffer[MAX_DATA_BYTES];

You can see it being supplied to the callback:

(*currentSysexCallback)(currentSysexCallbackContext, dataBuffer[0], sysexBytesRead - 1, dataBuffer + 1);

You are right, in saying it would be best practice to do so, and to protect against the buffering scheme being changed. For now I want a surgical fix, before I implement a much broader change to the firmware.

@rwaldron
Copy link
Contributor

This change would need to be made across all variants.

The current implementation pulls bytes out of the sysex buffer, regardless of whether or not the user specified such bytes.
@soundanalogous
Copy link
Member

lgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants