# HG changeset patch # User Matti Hamalainen # Date 1425747438 -7200 # Node ID 5ae2ce21c7aecaab5c8bac81402c0c29b3edec2e # Parent 7bd50496c9ec783eca3e9c4c9cad6ceb7c038ff9 Some work on refactoring the JSSMOD loader. diff -r 7bd50496c9ec -r 5ae2ce21c7ae minijss/jloadjss.c --- a/minijss/jloadjss.c Sat Mar 07 18:19:43 2015 +0200 +++ b/minijss/jloadjss.c Sat Mar 07 18:57:18 2015 +0200 @@ -17,10 +17,6 @@ // Short helper macros for reading data -#define JSGETBUF(XV, XT) \ - if (!dmf_read_str(inFile, XV, sizeof(XT))) \ - return DMERR_OUT_OF_DATA - #define JSGETBYTE(XV) \ if (!dmf_read_byte(inFile, XV)) \ return DMERR_OUT_OF_DATA @@ -263,32 +259,53 @@ #endif -#undef JSGETBUF -#undef JSGETBYTE -#define JSGETBUF(XV, XT) do { \ - if (!dmf_read_str(inFile, XV, sizeof(XT))) \ - JSSERROR(DMERR_OUT_OF_DATA, DMERR_OUT_OF_DATA, \ - "Out of data at getting " # XT " (%d bytes)\n", sizeof(XT)); \ -} while (0) -#define JSGETBYTE(XV) if (!dmf_read_byte(inFile, XV)) return DMERR_OUT_OF_DATA - - #ifdef JM_SUP_EXT_INSTR -static void jssCopyEnvelope(JSSEnvelope *e, JSSMODEnvelope *je) +static int jssMODLoadEnvelope(DMResource *inFile, + JSSEnvelope *env, const char *name, const int ninst) { + JSSMODEnvelope jssEnv; int i; - e->flags = je->flags; - e->npoints = je->npoints; - e->sustain = je->sustain; - e->loopS = je->loopS; - e->loopE = je->loopE; + // Read envelope data + if (!dmf_read_le16(inFile, &jssEnv.flags) || + !dmf_read_le16(inFile, &jssEnv.npoints) || + !dmf_read_le16(inFile, &jssEnv.sustain) || + !dmf_read_le16(inFile, &jssEnv.loopS) || + !dmf_read_le16(inFile, &jssEnv.loopE)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read %s-envelope data for instrument #%d.\n", + name, ninst); + + // Do some sanity checking + if (jssEnv.npoints > jsetMaxEnvPoints || + jssEnv.loopS < jssEnv.loopE || + jssEnv.loopS > jssEnv.npoints || + jssEnv.loopE > jssEnv.npoints) + JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA, + "Invalid values in %s-envelope for instrument #%d.\n", + name, ninst); - for (i = 0; i < je->npoints; i++) + // Copy data + env->flags = jssEnv.flags; + env->npoints = jssEnv.npoints; + env->sustain = jssEnv.sustain; + env->loopS = jssEnv.loopS; + env->loopE = jssEnv.loopE; + + for (i = 0; i < jssEnv.npoints; i++) { - e->points[i].frame = je->points[i].frame; - e->points[i].value = je->points[i].value; + Uint16 frame, value; + if (!dmf_read_le16(inFile, &frame) || + !dmf_read_le16(inFile, &value)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read %s-envelope values (%d) for instrument #%d.\n", + name, i, ninst); + + env->points[i].frame = frame; + env->points[i].value = value; } + + return DMERR_OK; } #endif @@ -297,13 +314,15 @@ { JSSModule *module; JSSMODHeader jssH; - int index; + int index, ret = DMERR_OK; *ppModule = NULL; // Check the JSSMOD header - dmMemset(&jssH, 0, sizeof(jssH)); - JSGETBUF(&jssH, JSSMODHeader); + if (!dmf_read_str(inFile, &jssH.idMagic, sizeof(jssH.idMagic)) || + !dmf_read_byte(inFile, &jssH.idVersion)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read JSSMOD header #1.\n"); if (memcmp(jssH.idMagic, "JM", 2) != 0) { @@ -324,6 +343,21 @@ jssH.idVersion, JSSMOD_VERSION); } + // Read rest of the header + if (!dmf_read_le16(inFile, &jssH.defFlags) || + !dmf_read_le16(inFile, &jssH.intVersion) || + !dmf_read_le16(inFile, &jssH.norders) || + !dmf_read_le16(inFile, &jssH.npatterns) || + !dmf_read_le16(inFile, &jssH.nextInstruments) || + !dmf_read_le16(inFile, &jssH.ninstruments) || + !dmf_read_le16(inFile, &jssH.defRestartPos) || + !dmf_read_byte(inFile, &jssH.nchannels) || + !dmf_read_byte(inFile, &jssH.defSpeed) || + !dmf_read_byte(inFile, &jssH.defTempo) || + !dmf_read_byte(inFile, &jssH.patMode)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read JSSMOD header #2.\n"); + if (probe) return DMERR_OK; @@ -362,16 +396,23 @@ // Get the orders list for (index = 0; index < module->norders; index++) { - Sint16 order; - JSGETBUF(&order, Sint16); - module->orderList[index] = order; + Uint16 tmp; + if (!dm_fread_le16(inFile, &tmp)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read orders list entry #%d.\n", + index); + + if (tmp != 0xffff && tmp > jssH.npatterns) + JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA, + "Invalid orders list entry #%d value %d > %d.\n", + index, tmp, jssH.npatterns); + } // Parse the patterns for (index = 0; index < module->npatterns; index++) { JSSMODPattern jssP; - int result = DMERR_INVALID_DATA; // Get header and check size dmMemset(&jssP, 0, sizeof(jssP)); @@ -391,27 +432,27 @@ { #ifdef JM_SUP_PATMODE_1 case PATMODE_RAW_HORIZ: - result = jssGetPatternRawHoriz(inFile, module->patterns[index]); + ret = jssGetPatternRawHoriz(inFile, module->patterns[index]); break; #endif #ifdef JM_SUP_PATMODE_2 case PATMODE_COMP_HORIZ: - result = jssGetPatternCompHoriz(inFile, module->patterns[index]); + ret = jssGetPatternCompHoriz(inFile, module->patterns[index]); break; #endif #ifdef JM_SUP_PATMODE_3 case PATMODE_RAW_VERT: - result = jssGetPatternRawVert(inFile, module->patterns[index]); + ret = jssGetPatternRawVert(inFile, module->patterns[index]); break; #endif #ifdef JM_SUP_PATMODE_4 case PATMODE_COMP_VERT: - result = jssGetPatternCompVert(inFile, module->patterns[index]); + ret = jssGetPatternCompVert(inFile, module->patterns[index]); break; #endif #ifdef JM_SUP_PATMODE_5 case PATMODE_RAW_ELEM: - result = jssGetPatternRawVertElem(inFile, module->patterns[index]); + ret = jssGetPatternRawVertElem(inFile, module->patterns[index]); break; #endif default: @@ -421,9 +462,9 @@ break; } - if (result != DMERR_OK) + if (ret != DMERR_OK) { - JSSERROR(result, result, + JSSERROR(ret, ret, "Error in unpacking pattern #%d data.\n", index); } @@ -437,9 +478,18 @@ JSSExtInstrument *einst; int i; - dmMemset(&jssE, 0, sizeof(jssE)); - JSGETBUF(&jssE, JSSMODExtInstrument); - + // Read header data + if (!dmf_read_byte(inFile, &jssE.nsamples) || + !dmf_read_byte(inFile, &jssE.vibratoType) || + !dmf_read_le16(inFile, &jssE.vibratoSweep) || + !dmf_read_le16(inFile, &jssE.vibratoDepth) || + !dmf_read_le16(inFile, &jssE.vibratoRate) || + !dmf_read_le16(inFile, &jssE.fadeOut)) + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read ext.instrument #%d header.\n", + index); + + // Allocate instrument if ((einst = jssAllocateExtInstrument()) == NULL) { JSSERROR(DMERR_MALLOC, DMERR_MALLOC, @@ -455,14 +505,33 @@ einst->vibratoRate = jssE.vibratoRate; einst->fadeOut = jssE.fadeOut; + // Read and somewhat validate sNumForNotes for (i = 0; i < jsetNNotes; i++) { - int snum = jssE.sNumForNotes[i]; - einst->sNumForNotes[i] = (snum > 0) ? snum - 1 : jsetNotSet; + int snum; + Uint32 tmp; + + if (!dmf_read_le32(inFile, &tmp)) + { + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read ext.instrument #%d sNumForNotes[%d].\n", + index, i); + } + + einst->sNumForNotes[i] = snum = (tmp > 0) ? ((int) tmp - 1) : jsetNotSet; + + if (snum != jsetNotSet && snum > module->ninstruments) + { + JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA, + "Ext.instrument #%d has invalid sNumForNotes[%d] = %d > %d max.\n", + index, i, snum, module->ninstruments); + } } - jssCopyEnvelope(&(einst->volumeEnv), &jssE.volumeEnv); - jssCopyEnvelope(&(einst->panningEnv), &jssE.panningEnv); + // Read and validate envelopes + if ((ret = jssMODLoadEnvelope(inFile, &einst->volumeEnv, "volume", index)) != DMERR_OK || + (ret = jssMODLoadEnvelope(inFile, &einst->panningEnv, "panning", index)) != DMERR_OK) + return ret; } #ifdef JM_SUP_INSTR @@ -472,27 +541,45 @@ JSSMODInstrument jssI; JSSInstrument *inst; - dmMemset(&jssI, 0, sizeof(jssI)); - JSGETBUF(&jssI, JSSMODInstrument); - - if ((inst = jssAllocateInstrument()) == NULL) + // Read header data + if (!dmf_read_le32(inFile, &jssI.size) || + !dmf_read_le32(inFile, &jssI.loopS) || + !dmf_read_le32(inFile, &jssI.loopE) || + !dmf_read_le16(inFile, &jssI.flags) || + !dmf_read_le16(inFile, &jssI.C4BaseSpeed) || + !dmf_read_le16(inFile, &jssI.ERelNote) || + !dmf_read_le16(inFile, &jssI.EFineTune) || + !dmf_read_le16(inFile, &jssI.EPanning) || + !dmf_read_byte(inFile, &jssI.volume) || + !dmf_read_byte(inFile, &jssI.convFlags)) { - JSSERROR(DMERR_MALLOC, DMERR_MALLOC, - "Could not allocate sample instrument structure #%d\n", index); + JSSERROR(DMERR_FREAD, DMERR_FREAD, + "Failed to read sample instrument #%d header.\n", + index); } - module->instruments[index] = inst; + // Validate what we can + if (jssI.loopS > jssI.size || + jssI.loopE > jssI.size || + jssI.loopE > jssI.loopS) + { + JSSERROR(DMERR_INVALID_DATA, DMERR_INVALID_DATA, + "Invalid or corrupted sample instrument #%d.\n", + index); + } + + // Allocate instrument // Copy data inst->size = jssI.size; inst->loopS = jssI.loopS; inst->loopE = jssI.loopE; - inst->volume = jssI.volume; inst->flags = jssI.flags; inst->C4BaseSpeed = jssI.C4BaseSpeed; inst->ERelNote = jssI.ERelNote; inst->EFineTune = jssI.EFineTune; inst->EPanning = jssI.EPanning; + inst->volume = jssI.volume; inst->convFlags = jssI.convFlags; }