summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfrosch <frosch@openttd.org>2010-01-03 19:29:56 +0000
committerfrosch <frosch@openttd.org>2010-01-03 19:29:56 +0000
commit666c856457ebda1a4261d0da95c11ecefc04708f (patch)
treeea9582ac8df5b3d68a9d1246f859c17c186d554f
parentf82c591022c37a2737afd040f65ab04e7af82da1 (diff)
downloadopenttd-666c856457ebda1a4261d0da95c11ecefc04708f.tar.xz
(svn r18699) -Fix [FS#PlanetAndy]: GRF parameters were not properly initialised to zero, and not always checked for valid range.
-rw-r--r--src/newgrf.cpp25
-rw-r--r--src/newgrf.h9
-rw-r--r--src/newgrf_spritegroup.cpp4
3 files changed, 27 insertions, 11 deletions
diff --git a/src/newgrf.cpp b/src/newgrf.cpp
index 7d21ba60b..0b6130a72 100644
--- a/src/newgrf.cpp
+++ b/src/newgrf.cpp
@@ -3863,7 +3863,7 @@ static uint32 GetParamVal(byte param, uint32 *cond_val)
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 @@ static void GRFLoadError(byte *buf, size_t len)
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 @@ static uint32 PerformGRM(uint32 *grm, uint16 num_ids, uint16 count, uint8 op, ui
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 @@ static void ParamSet(byte *buf, size_t len)
switch (op) {
case 2:
case 3:
- src1 = _cur_grffile->param[target];
+ src1 = _cur_grffile->GetParam(target);
break;
default:
@@ -4680,10 +4680,10 @@ static void ParamSet(byte *buf, size_t len)
/* 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 @@ static void ParamSet(byte *buf, size_t len)
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 @@ static void InitNewGRFFile(const GRFConfig *config, int sprite_offset)
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;
}
diff --git a/src/newgrf.h b/src/newgrf.h
index ea9b890a0..64b8f9172 100644
--- a/src/newgrf.h
+++ b/src/newgrf.h
@@ -118,6 +118,15 @@ struct GRFFile {
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 {
diff --git a/src/newgrf_spritegroup.cpp b/src/newgrf_spritegroup.cpp
index 74cba113b..348f8ebf0 100644
--- a/src/newgrf_spritegroup.cpp
+++ b/src/newgrf_spritegroup.cpp
@@ -62,8 +62,8 @@ static inline uint32 GetVariable(const ResolverObject *object, byte variable, by
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);