Am 09.10.19 um 03:19 schrieb Junio C Hamano: > Johannes Sixt <j6t@xxxxxxxx> writes: > >> We do not want to have it for *all* cases, where we return -1 - pos, but >> only for those cases, where the result was actually encoded by >> index_pos_to_insert_pos(). > > Yup, I agree with you that decoder should be fed only the data > emitted by the encoder. > > But shouldn't the code that yielded 'pos' that later gets decoded by > computing "-1 -pos" without using the encoding helper be corrected > to use the encoder instead? That is the obvious conclusion, of course. > After all, the primary purpose of > inventing the encoder was to catch the arith overflow, wasn't it? That was *your* motivation for the helper function. But IMO it is a wrong design decision. Whether or not the index calculation overflows is a matter of the type that is used for the index, and that in turn is dicated by the possible sizes of the collections that are indexed. IOW, the index overflow check is (*if* it is necessary) a policy decision that must be made at a higher level and must not be hidden away in a helper function whose purpose (as suggested by its name) is something entirely different. Unless, of course, we declare "all our indexes are of type int". But that ship has sailed long ago, because there are too many cases where we are forced to use size_t as index (strlen, sizeof...). Meta note: We know that we are painting a tiny shed here (Replacing a one-liner by a one-liner, huh?) If anyone of you has better things to do, please move on. My interest in this discussion are just the design decisions that are made, not the actual outcome of this particular case. -- Hannes
