Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
>>> This is obviously an improvement, but it makes me wonder if we should be
>>> doing:
>>>
>>> if (!check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data))
>>> die("BUG: some explanation of why this can never happen");
>>>
>>> which perhaps documents the intended assumptions more clearly. A comment
>>> regarding the side effects might also be helpful.
>>
>> I wondered exactly the same thing myself. I was hoping Jonathan would
>> pipe in here with some analysis about whether this is:
>>
>> a) a super paranoid, just-in-case, can't really ever fail because by
>> the time we get to this code we've already effectively validated
>> everything that could cause check_header to return false in this case
>> ...
> The answer is "a". The only time that mi->inbody_header_accum is
> appended to is in check_inbody_header, and appending onto a blank
> mi->inbody_header_accum always happens when is_inbody_header is true
> (which guarantees a prefix that causes check_header to always return
> true).
>
> Peff's suggestion sounds reasonable to me, maybe with an error message
> like "BUG: inbody_header_accum, if not empty, must always contain a
> valid in-body header".
OK. So we do not expect it to fail, but we still do want the side
effect of that function (i.e. accmulation into the field).
Somebody care to send a final "agreed-upon" version?