[janitorial] flashRepro_N64: sectorSize and bufferSize may be used uninitialised about cartreader HOT 5 CLOSED

There is another warning about an uninitialised variable in one of the functions called from there: writeFlashBuffer_N64.

In that case it is a bit clearer that this should not be an issue: the variable would be uninitialised if bufferSize is 0, which seem unlikely to happen (...if the caller takes care to never provide 0, but the caller has bigger issues).

from cartreader.

You can initialize them to a value of zero if it removes the warning. In the case that they are needed they get new values anyway in the two switch cases.

from cartreader.

Do they ? The switch blocks are only for the case where cartSize == 0 (the else branch). The only lines outside of that branch which touch these variables are:

void flashRepro_N64() {
  unsigned long sectorSize;
  byte bufferSize;
// ...
      } else if (bufferSize == 0) {
        writeFlashrom_N64(sectorSize);
      } else {
        writeFlashBuffer_N64(sectorSize, bufferSize);
      }

Also, setting to zero will cause another issue in writeFlashBuffer_N64. EDIT: this is incorrect, the code will go to writeFlashrom_N64 if bufferSize is zero. But there it will cause an infinite loop:

void writeFlashrom_N64(unsigned long sectorSize) {
// ...
  for (unsigned long currSector = 0; currSector < fileSize; currSector += sectorSize) {

sectorSize is zero, so the increment is zero, so currSector will never reach fileSize.

So I think there is more to fix here, but I do not know enough to tell exactly how it should be fixed.

from cartreader.

I see, maybe like this:

// ...
} else if ((bufferSize == 0) && (sectorSize != 0))  {
        writeFlashrom_N64(sectorSize);
} 
else if (sectorSize != 0) {
        writeFlashBuffer_N64(sectorSize, bufferSize);
}
else {
       print_Error(F("sectorSize not set"), true);
}

from cartreader.

Thanks, applied.

from cartreader.