Mercurial > hg > openttd
changeset 14151:899cf0332cda draft
(svn r18699) -Fix [FS#PlanetAndy]: GRF parameters were not properly initialised to zero, and not always checked for valid range.
author | frosch <frosch@openttd.org> |
---|---|
date | Sun, 03 Jan 2010 19:29:56 +0000 |
parents | 90cc587db2f1 |
children | 845717840ee5 |
files | src/newgrf.cpp src/newgrf.h src/newgrf_spritegroup.cpp |
diffstat | 3 files changed, 27 insertions(+), 11 deletions(-) [+] |
line wrap: on
line diff
--- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -3863,7 +3863,7 @@ default: /* GRF Parameter */ - if (param < 0x80) return _cur_grffile->param[param]; + if (param < 0x80) return _cur_grffile->GetParam(param); /* In-game variable. */ grfmsg(1, "Unsupported in-game variable 0x%02X", param); @@ -4380,7 +4380,7 @@ uint i = 0; for (; i < 2 && len > 0; i++) { uint param_number = grf_load_byte(&buf); - error->param_value[i] = (param_number < _cur_grffile->param_end ? _cur_grffile->param[param_number] : 0); + error->param_value[i] = _cur_grffile->GetParam(param_number); len--; } error->num_params = i; @@ -4493,11 +4493,11 @@ if (op == 6) { /* Return GRFID of set that reserved ID */ - return grm[_cur_grffile->param[target]]; + return grm[_cur_grffile->GetParam(target)]; } /* With an operation of 2 or 3, we want to reserve a specific block of IDs */ - if (op == 2 || op == 3) start = _cur_grffile->param[target]; + if (op == 2 || op == 3) start = _cur_grffile->GetParam(target); for (uint i = start; i < num_ids; i++) { if (grm[i] == 0) { @@ -4631,7 +4631,7 @@ switch (op) { case 2: case 3: - src1 = _cur_grffile->param[target]; + src1 = _cur_grffile->GetParam(target); break; default: @@ -4680,10 +4680,10 @@ /* Disable the read GRF if it is a static NewGRF. */ DisableStaticNewGRFInfluencingNonStaticNewGRFs(c); src1 = 0; - } else if (file == NULL || src1 >= file->param_end || (c != NULL && c->status == GCS_DISABLED)) { + } else if (file == NULL || (c != NULL && c->status == GCS_DISABLED)) { src1 = 0; } else { - src1 = file->param[src1]; + src1 = file->GetParam(src1); } } } else { @@ -4829,6 +4829,7 @@ default: if (target < 0x80) { _cur_grffile->param[target] = res; + /* param is zeroed by default */ if (target + 1U > _cur_grffile->param_end) _cur_grffile->param_end = target + 1; } else { grfmsg(7, "ParamSet: Skipping unknown target 0x%02X", target); @@ -5700,10 +5701,16 @@ newfile->price_base_multipliers[i] = INVALID_PRICE_MODIFIER; } - /* Copy the initial parameter list */ + /* Copy the initial parameter list + * 'Uninitialised' parameters are zeroed as that is their default value when dynamically creating them. */ assert_compile(lengthof(newfile->param) == lengthof(config->param) && lengthof(config->param) == 0x80); + memset(newfile->param, 0, sizeof(newfile->param)); + + assert(config->num_params <= lengthof(config->param)); newfile->param_end = config->num_params; - memcpy(newfile->param, config->param, sizeof(newfile->param)); + if (newfile->param_end > 0) { + MemCpyT(newfile->param, config->param, newfile->param_end); + } *_grf_files.Append() = _cur_grffile = newfile; }
--- a/src/newgrf.h +++ b/src/newgrf.h @@ -118,6 +118,15 @@ uint32 grf_features; ///< Bitset of GrfSpecFeature the grf uses PriceMultipliers price_base_multipliers; ///< Price base multipliers as set by the grf. + + /** Get GRF Parameter with range checking */ + uint32 GetParam(uint number) const + { + /* Note: We implicitly test for number < lengthof(this->param) and return 0 for invalid parameters. + * In fact this is the more important test, as param is zeroed anyway. */ + assert(this->param_end <= lengthof(this->param)); + return (number < this->param_end) ? this->param[number] : 0; + } }; enum ShoreReplacement {
--- a/src/newgrf_spritegroup.cpp +++ b/src/newgrf_spritegroup.cpp @@ -62,8 +62,8 @@ case 0x7D: return _temp_store.Get(parameter); case 0x7F: - if (object == NULL || object->grffile == NULL || parameter >= object->grffile->param_end) return 0; - return object->grffile->param[parameter]; + if (object == NULL || object->grffile == NULL) return 0; + return object->grffile->GetParam(parameter); /* Not a common variable, so evalute the feature specific variables */ default: return object->GetVariable(object, variable, parameter, available);